CODEWITHSUNDEEP
language-agnosticternary-operatorconditional-operator
nilesh
nilesh

Reputation: 1336

nested ternary operator vs nested if else, which is better in readability purpose

I found code in code review which was written by my team member. It contains nested ternary operator. I told him to use if else if there is more than one nesting for readability purpose. We had argue on that.

following is code

ColorEnum color = opacity == Opacity.FIVE? ColorEnum.BLACK : 
                opacity == Opacity.TEN? ColorEnum.WHITE :
                    opacity == Opacity.FIFTY? ColorEnum.RED :
                        opacity == Opacity.TWENTY? ColorEnum.BLUE :
                            opacity == Opacity.FIFTEEN? ColorEnum.PURPLE : null;

And this code is changing as new configurations come.

So What is better here? ternary operator or if else?

Upvotes: 4

Views: 2354

Answers (5)

haccks
haccks

Reputation: 106012

I would suggest to use switch statement. It would be more readable than ternary and if-else.

switch(opacity)
{
    case Opacity.FIVE: color = ColorEnum.BLACK;
    break;
    
    case Opacity.TEN: color = ColorEnum.WHITE;
    break;

    case Opacity.FIFTY: color = ColorEnum.RED;
    break;  

    ....

   default: printf("Error message\n");
}

Upvotes: 6

Marko Topolnik
Marko Topolnik

Reputation: 200158

Just reformatting your code makes it quite clear:

ColorEnum color = 
          opacity == Opacity.FIVE    ? ColorEnum.BLACK
       :  opacity == Opacity.TEN     ? ColorEnum.WHITE 
       :  opacity == Opacity.FIFTY   ? ColorEnum.RED 
       :  opacity == Opacity.TWENTY  ? ColorEnum.BLUE 
       :  opacity == Opacity.FIFTEEN ? ColorEnum.PURPLE 
       :  null;

LISP adopts the cond construct which has both the same structure and the same semantics, and is considered good practice. As an aside, Clojure also supports a form which tests the value of a single expression with a single predicate applied to different values (one for each clause) and calls it condp—that would be a perfect match for your use case.

The idiom with the ternary operator has the advantage over an if-else cascade for being an expression so you need only a single statement to assign it to the variable. if-else will force you to pull the assignment into each then clause, introducing more boilerplate and more opportunity to fail on correctness.

A switch statement could also be considered as an alternative, but it would have the following deficiencies:

  • like if-else, it is not an expression;

  • you are restricted to just different constant values of a single expression (the type of the expression being quite constrained, too).

  • it's prone to bugs due to the boilerplate break missing somewhere.

Upvotes: 7

Sebastian Negraszus
Sebastian Negraszus

Reputation: 12215

As others have mentioned, a switch would be a good alternative. To reduce boilerplate code (breaks, assignments), I would further recommend putting the switch into a dedicated method:

(All examples are in C#)

public ColorEnum OpacityToColor(Opacity opacity)
{
  switch (opacity)
  {
    case Opacity.FIVE:
      return ColorEnum.BLACK;
    case Opacity.TEN:
      return ColorEnum.WHITE;
    case Opacity.FIFTY:
      return ColorEnum.RED;
    case Opacity.TWENTY:
      return ColorEnum.BLUE;
    case Opacity.FIFTEEN:
      return ColorEnum.PURPLE;
    default:
      throw new System.ArgumentOutOfRangeException("opacity");
  }
}

// Elsewhere
ColorEnum color = OpacityToColor(opacity);

If your language has a neat dictionary/map initialization syntax (e.g. Python, C#), you could also use that for a very concise and clear notation:

public static readonly Dictionary<Opacity, ColorEnum> ColorByOpacity = 
  new Dictionary<Opacity, ColorEnum>
{
  {Opacity.FIVE, ColorEnum.BLACK},
  {Opacity.TEN, ColorEnum.WHITE},
  {Opacity.FIFTY, ColorEnum.RED},
  {Opacity.TWENTY, ColorEnum.BLUE},
  {Opacity.FIFTEEN, ColorEnum.PURPLE}
};

// Elsewhere
ColorEnum color = ColorByOpacity[opacity];

Upvotes: 1

Arun
Arun

Reputation: 2092

Go with switch.

I've a thumb rule which I consistently follow (Although not hardset)

1) Only one conditional evaluation, go with ternary operator

2) Two conditional check, go with if(){} else if(){ } else{} construct

3) Three or more go with switch ladder

“Programs must be written for people to read, and only incidentally for machines to execute.” ― Harold Abelson, Structure and Interpretation of Computer Programs

Upvotes: 1

lukfi
lukfi

Reputation: 319

For this purpose, probably a switch-case statement would be best in terms of readability.

Upvotes: 2

Related Questions