Is instantiating an array in condition a bad programming habit?

Question

I often see this kind of condition :

if ([CONSTANT_1, CONSTANT_2, CONSTANT_3].contains? var && a_boolean) #...

instead of :

if ((var == CONSTANT_1 || var == CONSTANT_2 || var == CONSTANT_3) var && a_boolean) #...

I understant that the first condition is more elegant, but does creating an array only to check a condition is bad or does the resource used are negligible enough to be ignored ?

A good compiler should be able to recognize that the array is static, and initialize it as a static variable (or whatever the equivalent is in Ruby). I wouldn't expect an array to actually be created every time the condition is run. — Carcigenicate, Jun 09 '17 at 15:47
@Carcigenicate I don't know if this would change your comment, but Ruby is interpreted, not compiled. — Piccolo, Jun 09 '17 at 15:56
I wouldn't worry about performance at all unless you app starts to feel slow. If it does, you'd probably find way more important stuff to optimize — Marko Avlijaš, Jun 09 '17 at 16:09
I would say it depends on what you are optimizing for: performance (a `Set` stored in a constant might be better), memory consumption (your version might be fine), readability or maintainability (the array version is a common short Ruby idiom)... — spickermann, Jun 09 '17 at 16:09
@Piccolo Whoops. Don't know Ruby, so I was making a generalized comment. If it's interpreted, it may actually create the list every time. With only a few elements though, it should be quick to create. Whether or not it's negligible hiwever would depend on how how often the condition is being run. — Carcigenicate, Jun 09 '17 at 16:11
Actually, it's not for my own app or project. I was just wondering since I often saw it on github project. — , Jun 09 '17 at 16:22

score 0 · Answer 1 · answered Jun 09 '17 at 18:39

The former pattern does exert more memory pressure and CPU overhead since it has to allocate that array each time this code is run.

That said, this isn't necessarily a problem unless it's a particularly hot code path. If this is a method that is called frequently deep in a library somewhere, I'd probably elect for the second version to avoid the extra allocations, but for business code with a relatively low call rate, I think the readability of the first is acceptable.

Kathryn · Answer 2 · 2017-06-09T19:31:28.630

The second version (like most complicated conditions) is error-prone. It's too easy to forget whether you need parentheses or how boolean logic works. The pattern of "is the variable one of these values and if so is this other thing true?" can be especially tricky, so it certainly makes sense to simplify.

However, I don't think the Array class includes contains?, does it? In my tests, I used include?:

CONSTANT_1 = 1
CONSTANT_2 = 2
CONSTANT_3 = 3.14
a_boolean = true
var = 3.14

puts "true" if [CONSTANT_1, CONSTANT_2, CONSTANT_3].include?(var) && a_boolean

Notice the parenthesis around var are necessary so that include? isn't searching for var && a_boolean instead of just var.

The only real problem I see with this idiom is that it would be a bit nicer to have the var come first. In SQL you might write something like:

var in (CONSTANT_1, CONSTANT_2, CONSTANT_3) and a_boolean

That seems just a little more natural to read.

If you are really worried about efficiency and you do this test many times, it might be better to set up a hash:

constant_test = {}
[CONSTANT_1, CONSTANT_2, CONSTANT_3].each do |k|
  constant_test[k] = true
end

puts "true" if constant_test[var] && a_boolean

This has the advantage of being a hash lookup instead of a loop each time you test. (See also: this answer.)

Charlie · Answer 3 · 2017-06-09T20:46:57.783

Ideally, you wouldn't see either one of those. Instead, you'd use something like this as would be more idiomatic and more testable by breaking the conditional into small pieces of functionality.

I realize this example isn't quite the same if you're using, but it's close enough to show what I mean.

class Thing
  def initialize(var)
    @var = var
  end

  def a_method
    return 'yes' if has_constaints? && boolean?
    'no'
  end

  private

  def has_constaints?
    @var == CONSTANT_1 || @var == CONSTANT_2 || @var == CONSTANT_3
  end

  def boolean?
    @var && a_boolean
  end
end

Is instantiating an array in condition a bad programming habit?

3 Answers3