From: M R Lemon Date: 2011-09-27T05:41:42+09:00 Subject: Re: Populate an array based on object --bcaec548a147a0ed3404adde2f57 Content-Type: text/plain; charset=ISO-8859-1 On 26 September 2011 20:22, Brian Candler wrote: > > def collect_urls(base_url, page_number) > > valid_urls = [] > > 1000.times do # prevent infinite looping > > text = get_text(base_url, page_number) > > if text =~ /\A\s*Previous/ > > valid_urls << "END!" > > break > > else > > valid_urls << @target_url > > page_number += 1 > > end > > end > > return valid_urls > > end > > Incidentally, you seem to be relying on the instance variable > @target_url being set after get_text being called. It would be cleaner > to return the url and the text as two values. > > return target_url, text > > ... > > target_url, text = get_text(base_url, page_number) Brian, Many thanks for your helpful suggestions. This is working for me now. Originally, I was trying to avoid 'hard-coding' the loop such as '1000.times do' but I see now that this is probably the most elegant solution. Two really good learning points there for me two on the use of instance variables and efficiently returning and referring to expressions. Thanks again! Matt --bcaec548a147a0ed3404adde2f57--