Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

How do I refactor this in Rails?

if formatted_address.blank? || country.blank? || lat.blank? || lng.blank?
  ...
end

If I have 10 attributes to check if blank, I don't wanna retype .blank? 10 times.

Thanks.

share|improve this question
1  
What are you trying to achieve via refactoring? –  Arthur P Oct 10 '12 at 15:36
    
If I have 10 attributes to check if blank, I don't wanna retype .blank? 10 times. –  Victor Oct 10 '12 at 15:43
    
From the maintainability standpoint, multiple ORs or || are the same and you should opt to list them all. Just come up with the proper formatting - in this case if your underlying objects changes in structure, you can quickly update the condition. At the same time you may want to have all these conditions compiled into an aggregate field within the parent entity (the one that has "formatted_address" properties) but I dont see a big point in it based on the context. –  Arthur P Oct 10 '12 at 15:49
    
@Arthur, thanks. –  Victor Oct 10 '12 at 15:55

2 Answers 2

up vote 3 down vote accepted

You could do this. You still have a list of attributes, in case you need to change them later

if [formatted_address, country, lat, lng].any? { |field| field.blank? }
  # ...
end

Edit: By the way, the above can be shortened to:

if [formatted_address, country, lat, lng].any?(&:blank?)
  # ...
end
share|improve this answer

You can use or instead of ||. In Ruby or and and are used for control flow. So

(expr1) or (expr2)

If expr1 is false then expr2 will be evaluated. This is similar to failsafe condition. In your case, all if conditions can be combined using or

formatted_address.blank? or country.blank? or lat.blank? or lng.blank?

With this, all conditions will be checked but if any one of them is true, we want to execute some code. That can be achieved by using and

formatted_address.blank? or country.blank? or lat.blank? or lng.blank? and do_something

If your do_something is small enough this approach is useful.

More information about or and and in Ruby is here

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.