How to avoid short-circuit evaluation on

Question

I'm working with Ruby on Rails and would like to validate two different models :

if (model1.valid? && model2.valid?)
...
end

However, "&&" operator uses short-circuit evaluation (i.e. it evaluates "model2.valid?" only if "model1.valid?" is true), which prevents model2.valids to be executed if model1 is not valid.

Is there an equivalent of "&&" which would not use short-circuit evaluation? I need the two expressions to be evaluated.

Oidea

Out of curiosity, why do you need both to be evaluated? Normally, if they are free of side effects, shortcircuiting is desirable. And why would #valid? have side effects? – Trubow

The two models are related to a same form : in the end, they'll have to be both validated to perform the action. There is no side effect, it's just because it's two models on a single form. – Oidea

Well... short-circuiting is exactly what you want. IF model1 is invalid, then why bother checking model2? – Trubow

To be fair, validating both models at once allows errors to be reported to the user on both models separately. It's annoying to only be notified about a validation error in model 2 only after you've fixed the model 1 error. – Karlmarxstadt

Indeed, but follow separation of responsibility. #valid? should only validate, not notify the user of errors. Hence, the "if" clause will be entered in all and only the cases when it needs to be. Then, the notification must be done separately from the validation - namely, inside the if clause. – Trubow

For reference, #valid? does NOT notify the user of errors. It only populates the models errors array with errors. The controller then shows the new/edit view again which then shows the errors. – Aintab

Answer 1

Try this:

[model1, model2].map(&:valid?).all?

It'll return true if both are valid, and create the errors on both instances.

Geraldo

Thanks! Great code! By the way, I discover the 'all?' method, and I didn't know that the 'map' method could be use with such a syntax ('&:valid?'). I will use this solution. – Oidea

@BrenoSalgado, why not? I think the opposite: this is a very ruby way of circumventing the short-circuit evaluation. (And I do, because: the use of map, &:method_name, and all?) – Avaunt

Use of map is superfluous, Enumerable#all? takes a block. – Coequal

Note that the map(&:valid?) step is important: [model1, model2].all?(&:valid?) is a short-circuit operation (it won't collect errors on model2 if there are errors on model1.) – Aphotic

To me this is more verbose and confusing than just setting the result of one valid? call to a variable first. If you don't want to set a variable and don't want to use the & operator on the basis that others might not get it, I'd say spend the extra two cents and call valid? twice in the array like malclocke does. – Infirm

On the other hand, if you have three or more valid? checks this could be worth it. It might also help to remove the outermost parentheses, I don't think they're necessary and it works for me without them. – Infirm

This is the best answer for me. Most Rubyish. Imagine an array of size you do not know in advance. Also you can't use all? as it short circuits on the first false, therefore you don't get the error messages on every model. – Strove

Answer 2

& works just fine.

irb(main):007:0> def a
irb(main):008:1> puts "a"
irb(main):009:1> false
irb(main):010:1> end
=> nil

irb(main):011:0> def b
irb(main):012:1> puts "b"
irb(main):013:1> true
irb(main):014:1> end
=> nil

irb(main):015:0> a && b
a
=> false

irb(main):016:0> a & b
a
b
=> false

irb(main):017:0> a and b
a
=> false

Karlmarxstadt

valid is a method that runs the validations on an ActiveRecord instance, populating the associated errors so that they can be reported back to the user. – Geraldo

Note that you have to be careful about the left-hand side of a & | or ^ operator, as they are methods defined on TrueClass and FalseClass only, AFAICT... – Thessa

Good point Mr. Matt - reporting errors on both models simultaneously is a worthy goal. Removed the "bad code smell" comment :) – Karlmarxstadt

@Mike: Interesting - I'll have to do more irb investigation. PS I love irb for investigating things like this! – Karlmarxstadt

Interesting! I never took a close look at & and | in logical contexts, since I automatically read those as bitwise operations. – Colyer

I like this, and I would do it if I am placing more emphasis on raising the bar vs. immediate readability, but I do think most programmers are unlikely to understand right away. I could see different people making different choices depending on their situation. – Infirm

Answer 3

How about:

if [model1.valid?,model2.valid?].all?
  ...
end

Works for me.

Victorie

Answer 4

Evaluate them separately and store the result in a variable. Then use a simple && between those booleans :)

Zany

Answer 5

One of the key concepts which allow making a code snippet more maintainable for a future developer is its expressiveness.

Let's consider the following examples:

([model1, model2].map(&:valid?)).all?

or

[model1.valid?,model2.valid?].all?

Both of them do their job well, but when a future developer will encounter any of them without seeing any explanatory comments, docs of your intent, or without reaching you directly, this developer will modify them having no idea that the purpose was to avoid the short-circuit evaluation.

Things become even worse when you have no tests.

This is why I suggest introducing a small wrapper method which will make all things clear immediately.

def without_short_circuit_evaluation(*conditions)
  conditions.all?
end

And later somewhere in your codebase.

if without_short_circuit_evaluation(model1.valid?, model2.valid?)
  # do something
end

Autochthon

Answer 6

I had a similar issue where I needed to check if two string weren't blank. Short circuit was preventing me to check both strings but I found out that in ruby we can use if not or unless

return nil if not string1.blank? && string2.blank?

or

return nil unless string1.present? && string2.present?

Orlov

Answer 7

Instead of creating an extra array with map, you can pass a block to all?.

[model_instance_1, model_instance_2].all? {|i| i.valid? }

Glyph

Wow, while not being as short as Matt's answer, this one is valid and lot better than &. – Aintab

Why does this answer have down-votes on it? Is there something wrong with this answer that isn't immediately obvious? – Dogtrot

@Paul Performance and obscurity. Symbol#to_proc was a lot slower than passing a block on older versions of ruby. (Not really an issue with 2 elements, though.) It's a relatively new addition to the core library leveraging old but not commonly used type coercion syntax. – Colyer