Sander Land wrote:
> 
> Here's my shot at rewriting your program.
> Note that this is simply how I would write it, not everything changed
> is necessarily an 'improvement'.

Hi

I do appreciate the feedback!

> require 'open-uri'
> def random_date(years_back=5)
>  Time.at(Time.now - rand(years_back * 365 * 24 * 3600)).strftime('%y%m%d')
> end

ah yes, I thought about that after I posted this. Didn't know about
strftime, hence my C sprintf standby :-) I may slightly adjust this
to start with a date in 1995 and work forward to the present.

> r_date=random_date(10)
> puts r_date
> 
> open("http://antwrp.gsfc.nasa.gov/apod/ap#{r_date}.html") {|f|
>  text = f.read
>  if text =~ /<TITLE>([^<]*)/i
>   puts "----------------------------------------------"
>   puts $1.strip
>   puts "----------------------------------------------\n\n"
>  end
> 
>  if text =~ /href=\"([^\"]+(jpg|gif))\">/i
>    image = $1
>    fetch_url = "http://apod.nasa.gov/apod/" + image
>    puts fetch_url
>    # system(...)
>  end
> }

nice ...

> Comments:
> 
> - your random_date generated invalid dates sometimes, 01/30/02 and
> such. Using timestamps for everything is a bit more stable (Although
> slightly off if lots of leap years are in the range).
> - puts does to_s automatically,  so printf("%s\n",..) is never needed
> - reading the text per line was meant as an optimization? This is
> probably not necessary.

no, not really, just thought the string matching would work
better with line-by-line.

> - $' / $` and such are rarely used in Ruby, try to avoid them when the
> more readable $1 or String#scan will do.
> - more methods / classes are not really needed for such a small program.

Thanks for taking time to make these suggestions/comments, they are
very useful and just exactly what I was looking for.

Esmail