Pregunta

¿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 mejor me voy con el clásico ...

if (color != null) {
    messageColor = color;
}
¿Fue útil?

Solución

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;

Que en algunos idiomas se evaluará como "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 coherente para que la siguiente persona que lea su código (incluso si es usted) tenga una sobrecarga cognitiva mínima.

Otros consejos

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

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

que es un poco estúpido.

Normalmente escribiría el segundo en una línea, pero esa es una cuestión de resp. pautas de estilo de codificación:

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

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

Como está buscando 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);
    }
}

El uso del operador ternario es a menudo un tema delicado, junto con otros estándares de codificación. Es probable que 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 reasignar messageColor a sí mismo, por lo que la única función del operador ternario en esta situación particular es la ofuscación de código.

El operador ternario es más común entre los programadores de C. En C, si evita las estructuras de control, a menudo puede obtener una mejor canalización, ya que no hay predicción de ramales que salga mal. Dudo que vea alguna diferencia de rendimiento en Java, y el patrón if-null-then-asignar 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 una función defaultIfNull , firstNonNull o coalesce que puede hacer que el código sea aún más conciso. Apache Commons Lang incluye una función defaultIfNull .

Algunos idiomas incluyen un operador || = , que es el idioma habitual para valores predeterminados en esos idiomas.

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 deja esto tan claro.

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

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

Los operadores ternarios a menudo son abusados ??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 recomendable utilizar la versión más larga de

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

En lugar de una sintaxis ternaria.

Me parece bien (uso 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.

Licenciado bajo: CC-BY-SA con atribución
No afiliado a StackOverflow
scroll top