Tell me more ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

The following logic:

Iterate over the URLs array and If the HTTP request with the given URL does not timeout, continue. If the URL times out, remove it from the array and continue with the next one until the array is exhausted.

Has been implemented in the following way:

@urls.delete_if do |url|
  begin
    doc = perform_request(some_params)
    break
  rescue TimeoutError
    Rails.logger.warn("URL #{url} times out, will be removed from list")
    true
  end
end

Anyone for a cleaner solution? Basically, I want to iterate over an array and remove elements from it if there is a timeout, but if the URL works loop should end and app should continue processing.

share|improve this question
Specifically, hate the break in there - maybe someone can propose a cleaner solution. – Emo Jan 4 at 15:23
Btw "true" in the rescue can be removed, not needed. – Emo Jan 4 at 15:33
if not needed just edit the question! I have a doubt, why do you want to modify inplace @urls? wouldn't do a functional solution? – tokland Jan 4 at 17:57

2 Answers

You may use drop_while to get rid of iterative operator break:

@urls = @urls.drop_while do |url|
  begin
    doc = perform_request(some_params)
    false
  rescue TimeoutError
    Rails.logger.warn("URL #{url} times out, will be removed from list")
    true
  end
end
share|improve this answer
the problem is doc won't be visible outside the block's scope. – tokland Jan 4 at 21:23

I'd take a functional approach (no in-place updates):

url, doc = @urls.map_detect do |url|
  begin
    [url, perform_request(some_params)]
  rescue TimeoutError
    Rails.logger.warn("URL #{url} timed out")
    false
  end
end

This snippet uses the custom abstraction Enumerable#map_detect (see also Facets)

You can then use url to update @urls, but it's better not to mix the algorithm (even if it's so simple) with the updates required by the app.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.