RSS

#21 Avoid Ternary Conditional Operator (? :)

01 Nov

There is a Java conditional operator, known as the ternary conditional operator, which is often used in entirely inappropriate situations that make poor code.  Abuse of this operator is so common that I advise avoiding it completely unless you know really what it is doing.  Actually there are cases where it works OK, but in many situations it creates larger, slower code.

I am assuming that you understand the basics:  <cond exp> ? <result 1> : <result 2>    The conditional expression is tested, and if true result 1 is returned, otherwise result 2 is returned.  Programmers often use this because it is visually terse, and it looks more sophisticated than an if-then-else statement.

Here is an example of code I found “in the wild” that is an example of a poor statement:

out.write(((task.getDueDate()==0)?"":" due date:"));

What this is doing is calling a method getDueDate, and checking if that is zero.  If not it writes out a prompt for due date (into a web page in this case), but if so, it !?!?! writes out a null string into to the web page.  This is functionally equivalent to:

if (task.getDueDate()==0) {
    out.write("");
} else {
    out.write(" due date:");
}

Obviously, the first form is written in a single line, making it more compact.  Sometimes being more compact is good, and sometimes it can be bad, so I don’t want to focus on that at this time.  Instead, please note that when the due date is zero, it makes a completely unnecessary call to the write method, with a null string which produces no output.  It is far better to not call that at all.  This is better way to write this:

if (task.getDueDate()!=0) {
    out.write(" due date:");
}

That is the correct way to write this logic.  It is clear, easy to read, unambiguous, and it does not cause any extra, unnecessary operations to be called.

Actually, the story gets more interesting, because this is clearly a prompt, there is clearly going to be a value displayed after this.  The original code was something like this:

out.write(((task.getDueDate()==0)?"":" due date:"));
writeValue(out, formatDate(task.getDueDate()));

That second line is not needed in the case that the due date is zero, and we must assume that the formatDate function returns a null string when the date is zero.  But note that formatDate is called even when we already know that the due date has been tested for zero.  And then writeValue is called as well will a null string.  All of this is unnecessary.

A far better way to write this is:

if (task.getDueDate()!=0) {
    out.write(" due date:");
    writeValue(out, formatDate(task.getDueDate()));
}

The point being of course that you do not need to call format date if you already know that the due date is zero, and you don’t need to call write value if the know that the due date is zero.  The above, of course, can not be accomplished with the ternary conditional expression, and that is the second problem: programmers who use the ternary form believe that because it is visually compact, that the resulting code is compact, and don’t see the big picture.

There is one more improvement that I will include, because the above code requires two calls to the getDueDate function, and it would be better to have one and save the result in a local variable.  Here is the best form:

long dueDate = task.getDueDate();
if (dueDate!=0) {
    out.write(" due date:");
    writeValue(out, formatDate(dueDate));
}

Once again, this is readable, clear, unambiguous.  It does not look clever and sophisticated the way that the ternary operator does.  But remember, sophisticated code is not necessarily better.  Simple code is easier to read and maintain.

In Defense of the Terniary Operator

I have presented above a “bad example” of the ternary operator.  Are all examples bad?  No, there are cases where use of the ternary operator are OK.  That is it, just OK.  It is acceptable to use the ternary operator in a case like this:

int a = (b>c) ? f(b) : f(c);

Here we have an assignment statement, and you have a condition to choose between two values.  Either you want a to equal f(b), or to equal f(c), but one of these values needs to be assigned.  This is equivalent to:

int a;
if (b>c) {
    a = f(b);
} else {
    a = f(c);
}

This is visually more verbose.  In this extremely simple example, and one in which both the true and the false branch need to be executed, it is OK to use the ternary operator.  But keep in mind that the ternary operator is NOT more efficient either.  It is just dangerous, and if you know what you are doing, use it with care.  But less senior programmers should simply avoid this kind of statement.

Nuggets

If the condition is simple, and the results expressions are simple, then the ternary operator is OK

If your expression is more complex, or the result expressions are complex, then the ternary operation is less easy to read and understand.

If you need to do multiple statements in the branches, then the ternary operator will lead you to less than optimal solutions.

Advertisements
 
7 Comments

Posted by on November 1, 2011 in Coding

 

Tags: ,

7 responses to “#21 Avoid Ternary Conditional Operator (? :)

  1. kswenson

    March 23, 2012 at 6:13 pm

    I was reviewing some code now, and found this line:


    bpmnFile = bpmnFile == null || bpmnFile.equals("") ? bpmnPropertiesUtil.getBPMNFile() : bpmnFile;

    Horrible! The variable bpmnFile was a parameter to the method. Instead, this should simply be written as:


    if (bpmnFile == null || bpmnFile.equals("")) {

      bpmnFile = bpmnPropertiesUtil.getBPMNFile();

    }

    Far easier to read, more efficient, and much more maintainable. I believe the coder simply thought the ternary operator was more “sophisticated”

     
  2. freekdb

    August 2, 2017 at 11:57 am

    For you last example, this statement can be used as well:

    int a = f(Math.max(b, c));

     
  3. Nick Laros

    July 17, 2018 at 5:29 pm

    dont agree. ternary operator is so much useful if it used right. for example: writing in functional programming style

     
    • kswenson

      July 17, 2018 at 8:34 pm

      I can believe that people who get used to it may find it more familiar and more comfortable, but that is just whatever you are used to. You offer no evidence that it is objectively better. My article gives objective cases where the operation is abused, and I also admit of some cases where it is reasonable. It would be nice if commenters actually contributed to the information, and not just state opinions.

       
  4. retribution1979

    September 17, 2018 at 9:27 pm

    Anyone that reads this article should probably disregard it, as ironically, the article itself endorses bad coding practices. Seems like whoever wrote this article is new to coding, and doesn’t truly understand why overly verbose code isn’t a good thing. It’s actually harder to read/process code like:

    if (task.getDueDate()==0) {
    out.write(“”);
    } else {
    out.write(” due date:”);
    }

    vs:

    out.write( ( ( task.getDueDate()==0) ? “” : ” due date:” ) ) ;

    please OP, elaborate on why you think 5 lines of code is somehow more readable than 1?

    Any coder worth his salt can easily take a simple glance at the latter example and grasp what’s going on.

    Please, do potential newbies that may come across your articles a favor, and stop writing articles about practices you don’t understand. That’s how we end up with a market full of coders who write shit code, because they took bad advice from someone who didn’t know what they were talking about.

     
    • kswenson

      January 9, 2019 at 8:47 am

      The reason, *retribution1979*, is because your example shows exactly how people abuse the pattern for no reason. Your “bad” example is:

      if (task.getDueDate()==0) {
      out.write(“”);
      } else {
      out.write(” due date:”);
      }

      However, what is stupid about this example is that second line, where you write out a null string. This involves actually calling the write method, and passing to it a string with no characters in it. This would be better written as:

      if (task.getDueDate()!=0) {
      out.write(” due date:”);
      }

      Now this, even though it is three lines, is more readable than one line. Readability is not just the number of lines, but the complexity of the lines.

      You could have come up with a better example, one that writes out two different strings. There are some special cases where the ternary operator will be justified. In my experience though, programmers will tend to abuse the pattern, and write code that is LESS efficient than if they simple wrote the logic in a set of lines. You example, for instance:

      out.write( ( ( task.getDueDate()==0) ? “” : ” due date:” ) ) ;

      is inefficient because it will ALWAYS call the write method, even when it does not need to. THAT is the problem that you run into. This example calls the write method with a string with no characters in it, and the programmer (retribution1979) does not seem to be aware that he did this.

      If you are hung up on single lines, I could also write the following:

      if (task.getDueDate()!=0) {out.write(” due date:”);}

      I would like to point out that this is easily as readable as the ternary operator version AND it is more efficient because it only calls write when it needs to.

      Hopefully the readers won’t be fooled by charlatans who claim to know something about coding.

       
      • retribution1979

        July 2, 2019 at 5:03 pm

        Uhm, clearly in your example you post the following code:

        if (task.getDueDate()==0) {
        out.write("");
        } else {
        out.write(" due date:");
        }

        I quoted the article directly. In my example, it achieves the same thing in 1 line vs the 5 shown.

        I reckon based on your example of only calling the write method under x condition, you could also do:

        task.getDueDate() == 0 ? out.write(‘due date’) : “”

        Which would only run it if the operator evaluates to true, is only 1 line of code, and functions exactly as you described.

        I’ve been coding professionally for a VERY long time, and am fully self taught and know SEVERAL programming langues at a masters level. I guarantee I could code circles around you which is obvious given your lack of understanding of my response in relation to the article.

         

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out /  Change )

Google photo

You are commenting using your Google account. Log Out /  Change )

Twitter picture

You are commenting using your Twitter account. Log Out /  Change )

Facebook photo

You are commenting using your Facebook account. Log Out /  Change )

Connecting to %s

 
%d bloggers like this: