1

A co-worker submitted code for a PR the other day, which effectively follows this flow:

if (a)
  do something_a
else if (b)
  do something_b
else if (c)
  do something_a

where a, b, and c are mutually exclusive conditions (in reality, they are all checking if a variable is equal to different values).

In my experience however, I've always tried to collapse the conditionals that have the same results with ORs, like the following.

if (a OR c)
  do something_a
else if (b)
  do something_b

Disregarding performance, which version is better? Which version is easier to maintain in the future?

Philip Kendall
  • 22,899
  • 9
  • 58
  • 61
JRJurman
  • 347
  • 1
  • 10
  • 2
    "they are all checking if a variable is equal to different values" - is this in a language which has a `switch` type construct? – Philip Kendall Jul 25 '17 at 06:34
  • it is, but a, b, and c are methods which effectively do the check, otherwise yes, a switch-case would be a nice alternative. – JRJurman Jul 25 '17 at 06:42
  • Possible duplicate of [How would you know if you've written readable and easily maintainable code?](https://softwareengineering.stackexchange.com/questions/141005/how-would-you-know-if-youve-written-readable-and-easily-maintainable-code) – gnat Jul 25 '17 at 08:21

3 Answers3

9

At least as the question was originally asked, the two aren't equivalent.

In the first, something_a is executed only if a is true or if b is false and c is true. I.e., if (a || (c && !b)).

In the second, something_a is performed if either a or c is true (regardless of b). Only if those are both false do we consider b.

As such, your first question shouldn't be which is easier to maintain, but which behavior you actually want.

If the conditions are exclusive, so that difference doesn't arise, then I'd consider the relationship between the conditions, if any. If it's basically coincidence that two inputs happen to produce the same output, then I'd probably list each individual, since it's probably easier to simply map from inputs to results when each is listed directly. The second becomes interesting primarily when/if there's a fairly obvious relationship between the conditions.

For example, if you had something like:

if (customer has no ID || customer's age < minimum)
    reject order for alcoholic beverage

In this case, we have two different manifestations of what's really a single condition: the customer can't prove they're old enough to buy the drink. They're not (from our perspective) two unrelated conditions that happen to lead to the same result--rather, they're just different ways of expressing the same condition, so it makes more sense to group them together.

Jerry Coffin
  • 44,385
  • 5
  • 89
  • 162
1

Duplication is bad, therefore your first version is inferior.

How much worse it is depends on how much stuff do_stuff_a() actually is; if it's really just one line, then it's no big deal, but repeated groups of statements should generally be combined or refactored into a method.

Kilian Foth
  • 107,706
  • 45
  • 295
  • 310
-1

Neither is 'better' its just coding style.

(Disregarding both performance and whether they have the same result!!!!!)

Ewan
  • 70,664
  • 5
  • 76
  • 161