Pregunta Usando operador ternario con 4 expresiones


¿Es esta una práctica de codificación aceptable?

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
    }
}

o estoy mejor yendo con el clásico ...

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

11
2017-11-29 10:28


origen


Respuestas:


El uso del operador?: Debe limitarse para que el código sea más legible. Un ejemplo clásico:

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

En este caso, el código sería menos legible si lo dividiera en aproximadamente 5 líneas if / else.

Generalmente pongo corchetes alrededor de todo el operador para que cuando lo lea lo analice mentalmente como un valor único.

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

Otra variante es

messageColor = color || messageColor;

En algunos idiomas se evaluará "color", a menos que el color se evalúe como "falso", en cuyo caso el valor de messageColor. En mi opinión, esto debe evitarse ya que puede confundir a las personas.

Lo más importante es ser constante para que la siguiente persona que lea tu código (incluso si eres tú) tenga una carga cognitiva mínima.


24
2017-11-29 10:45



La legibilidad, la facilidad de comprensión, etc. son las mismas en este caso (Ya pues...) No me gusta la duplicación y la auto asignación aparente en el primer ejemplo; se traduciría a algo así como:

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

que es un poco estúpido

Por lo general, escribo el segundo en una línea, pero esa es una cuestión de resp individual de lujo. pautas de estilo de codificación:

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

EDITAR (Ahora soy más obstinado que hace 8 años)

Como busca las mejores prácticas:

// 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);
    }
}

4
2017-11-29 11:39



El uso del operador ternario es a menudo un tema delicado, junto con otros estándares de codificación. Probablemente su uso esté mejor determinado por los estándares de codificación en su sitio.

Sin embargo, en esta situación específica, definitivamente recomendaría la segunda opción; no solo es más claro, sino que el uso del operador ternario es totalmente innecesario aquí. No es necesario volver a asignar messageColor a sí mismo, por lo que la única función del operador ternario en esta situación particular es la ocultación del código.


2
2017-11-29 10:37



El operador ternario es más común entre los programadores C. En C, si evita las estructuras de control, a menudo puede obtener un mejor canalización, ya que no existe una predicción de bifurcación que salga mal. Dudo que veas diferencia de rendimiento en Java, y el patrón if-null-then-assign es mucho más común que el ternario. Sin embargo, si mantiene una base de código existente, generalmente es mejor mantenerse coherente con el código existente.

Si te encuentras haciendo esto mucho, puedes escribir un defaultIfNull, firstNonNull o coalesce función que puede hacer que el código sea aún más conciso. Apache Commons Lang incluye un defaultIfNull función.

Algunos idiomas incluyen un ||= operador, que es la expresión habitual para los valores predeterminados en esos idiomas.


2
2017-11-29 10:42



Prefiero el segundo porque expresa más claramente lo que quieres decir: solo quieres cambiar el color si no es nulo. El primer método no lo hace tan claro.


1
2017-11-29 10:33



En su caso, preferiría la implementación 'clásica', porque para mí es más rápido entender que solo quiere usar un nuevo color si la persona tiene uno preferido.

A veces lo uso en llamadas a métodos si quiero evitar los NPE, pero por lo general elijo esos feos fragmentos de código en una de las siguientes refactorizaciones;)


1
2017-11-29 11:25



Los operadores ternarios suelen ser objeto de abuso ya que el código que producen parece inteligente y compacto.

De hecho, hacen que el código sea menos legible y más propenso a errores. Siempre que sea posible, es aconsejable usar la versión más larga de

 if ( <condition> ) {
     <action> ;
 }

En lugar de una sintaxis ternaria.


1
2017-11-29 10:38



Me parece bien (utilizo mucho el operador ternario de Python), pero este tipo de problema de estilo suele ser muy subjetivo. Si el proyecto tiene un documento de estilo de codificación, es posible que desee verificarlo.


0
2017-11-29 10:33