Using ternary operator with 4 expressions

Question

Is this an acceptable coding practice?

public class MessageFormat {
    private static final Color DEFAULT_COLOR = Color.RED;

    private Color messageColor = DEFAULT_COLOR;

    public MessageFormat(Person person) {
        Color color = person.getPreferredColor();
        messageColor = (color != null) ? color : messageColor; // this line
    }
}

or am I better off going with the classic ...

if (color != null) {
    messageColor = color;
}

Answer 1

Use of the ?: operator should be confined to make code more readable. A classic example:

a = sprintf( "There are %i green bottle%s on the wall.", i, (i==1?"":"s") );

In this case the code would be less readable if you broke it up into about 5 if/else lines.

I generally put brackets around the entire operator so that when reading it I mentally parse it as a single value.

 messageColor = (color != null ? color : messageColor); 

Another variant is

messageColor = color || messageColor;

Which in some languages will evaluate to "color, unless color evaluates to "false", in which case value of messageColor. In my opinion this should be avoided as it may confuse people.

The most important thing is to be consistent so the next person reading your code (even if it's you) has minimum cognitive overhead.

Answer 2

Readability, ease of understanding etc. are the same in this case (I mean, come on...). I don't like the duplication and apparent self assignment in the first example; it would translate to something like:

if (colour != null) {messageColour = colour;}
   else {messageColour = messageColour;};

which is a bit stupid.

I would usually write the second in one line, but that's a question of individual fancy resp. coding style guidelines:

if (colour != null) {messageColour = colour;};

EDIT (I am now more opinionated than 8 years ago)

Since you are looking for best practices:

// Use default visibility by default, especially in examples.
// Public needs a reason.
class MessageFormat {
    static final Color DEFAULT_COLOR = Color.RED;

    // Strongly prefer final fields.
    private final Color messageColor;

    // Protect parameters and variables against abuse by other Java developers
    MessageFormat (final Person person) {
        // Use Optionals; null is a code smell
        final Optional<Color> preferredColor = person.getPreferredColor();
        // Bask in the clarity of the message
        this.messageColor = preferredColor.orElse(DEFAULT_COLOR);
    }
}