¿Está mal usar un parámetro booleano para determinar los valores?

El diseño de la API debe centrarse en lo que es más útil para un cliente de la API, desde el lado llamante .

Por ejemplo, si esta nueva API requiere que la persona que llama escriba con regularidad un código como este

if(flag)
    foo.setStateTrue();
else
    foo.setStateFalse();

entonces debería ser obvio que evitar el parámetro es peor que tener una API que permita a la persona que llama escribir

 foo.setState(flag);

La versión anterior solo produce un problema que luego debe resolverse en el lado de la llamada (y probablemente más de una vez). Eso no aumenta la legibilidad ni la facilidad de mantenimiento.

El lado de la implementación , sin embargo, no debe dictar cómo se ve la API pública. Si una función como setHint con un parámetro necesita menos código en la implementación, pero una API en términos setHintOn / setHintOff parece más fácil de usar para un cliente, se puede implementar de esta manera:

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

Entonces, aunque la API pública no tiene ningún parámetro booleano, aquí no hay lógica duplicada, así que solo hay un lugar para cambiar cuando llega un nuevo requisito (como en el ejemplo de la pregunta).

Esto también funciona al revés: si el método setState de arriba necesita cambiar entre dos partes de código diferentes, esas partes de código se pueden refactorizar a dos métodos privados diferentes. Entonces, en mi humilde opinión, no tiene sentido buscar un criterio para decidir entre "un parámetro / un método" y "cero parámetros / dos métodos" mirando las partes internas. Sin embargo, observe la forma en que le gustaría ver la API en el rol de un consumidor de la misma.

Si tiene dudas, intente usar "desarrollo guiado por pruebas" (TDD, por sus siglas en inglés), eso lo obligará a pensar en la API pública y cómo usarla.

Martin Fowler cita a Kent Beck en recomendando setOn() setOff() métodos , pero también dice que esto no debería ser considerado inviolable:

  

Si extrae datos [sic] de una fuente booleana, como un control de UI o una fuente de datos, prefiero tener setSwitch(aValue) que

if (aValue)
  setOn();
else
  setOff();

     

Este es un ejemplo de que se debe escribir una API para que sea más fácil para la persona que llama, por lo que si sabemos de dónde proviene la persona que llama, deberíamos diseñar la API teniendo en cuenta esa información. Esto también sostiene que a veces podemos proporcionar ambos estilos si recibimos llamadas de ambas formas.

Otra recomendación es usar un valor enumerado o un tipo de marca para Proporcione a true y false mejores nombres específicos para el contexto. En su ejemplo, showHint y hideHint podrían ser mejores.

Creo que estás mezclando dos cosas en tu publicación, la API y la implementación. En ambos casos, no creo que haya una regla firme que puedas usar todo el tiempo, pero deberías considerar estas dos cosas de manera independiente (tanto como sea posible).

Comencemos con la API, ambos:

public void setHint(boolean isHintOn)

y:

public void setHintOn()
public void setHintOff()

son alternativas válidas en función de lo que se supone que su objeto debe ofrecer y cómo sus clientes van a utilizar la API. Como señaló el Doc, si los usuarios ya tienen una variable booleana (desde un control de UI, una acción de usuario, una API externa, etc.), la primera opción tiene más sentido, de lo contrario, solo está forzando un extra if en el código del cliente. . Sin embargo, si, por ejemplo, está cambiando la sugerencia a verdadero al comenzar un proceso y a falso al final, la primera opción le ofrece algo como esto:

setHint(true)
// Do your process
…
setHint(false)

mientras que la segunda opción te ofrece esto:

setHintOn()
// Do your process
…
setHintOff()

que IMO es mucho más legible, por lo que elegiré la segunda opción en este caso. Obviamente, nada le impide ofrecer ambas opciones (o más, podría usar una enumeración como dijo Graham si eso tiene más sentido, por ejemplo).

El punto es que debes elegir tu API según lo que se supone que debe hacer el objeto y cómo los clientes lo van a usar, no según cómo lo vas a implementar.

Luego debes elegir cómo implementar tu API pública. Digamos que elegimos los métodos setHintOn y setHintOff como nuestra API pública y comparten esta lógica común como en su ejemplo. Podría abstraer fácilmente esta lógica a través de un método privado (código copiado de Doc):

private void setHint(boolean isHintOn){
    this.layer1.visible=isHintOn;
    this.layer2.visible=!isHintOn;
    this.layer3.visible=isHintOn;
}

public void setHintOn(){
   setHint(true);
}

public void setHintOff(){
   setHint(false);
}

A la inversa, supongamos que seleccionamos setHint(boolean isHintOn) a nuestra API, pero invirtamos su ejemplo, debido a cualquier motivo, la configuración de la sugerencia de activación es completamente diferente a la de desactivación. En este caso podríamos implementarlo de la siguiente manera:

public void setHint(boolean isHintOn){
    if(isHintOn){
        // Set it On
    } else {
        // Set it Off
    }    
}

O incluso:

public void setHint(boolean isHintOn){    
    if(isHintOn){
        setHintOn()
    } else {
        setHintOff()
   }    
}

private void setHintOn(){
   // Set it On
}

private void setHintOff(){
   // Set it Off 
}

El punto es que, en ambos casos, primero elegimos nuestra API pública y luego adaptamos nuestra implementación para adaptarla a la API elegida (y las restricciones que tenemos), no al revés.

Por cierto, creo que lo mismo se aplica a la publicación que vinculó sobre el uso de un parámetro booleano para determinar el comportamiento, es decir, debe decidir en función de su caso de uso particular en lugar de alguna regla difícil (aunque en ese caso particular normalmente lo correcto es dividirlo en varias funciones).

Lo primero es lo primero: el código no es automáticamente menos mantenible, solo porque es un poco más largo. La claridad es lo que importa.

Ahora, si realmente solo trata con datos, entonces lo que tiene es un configurador para una propiedad booleana. En ese caso, es posible que desee almacenar ese valor directamente y derivar las visibilidades de la capa, es decir,

bool isBackgroundVisible() {
    return isHintVisible;
}    

bool isContentVisible() {
    return !isHintVisible;
}

^{(Me he tomado la libertad de dar nombres reales a las capas; si no tiene esto en su código original, empezaría por eso)}

Esto todavía te deja con la pregunta de si se debe tener un método setHintVisibility(bool) . Personalmente, recomendaría reemplazarlo con los métodos showHint() y hideHint() ; ambos serán muy simples y no tendrá que cambiarlos cuando agregue capas. Sin embargo, no es un corte claro correcto / incorrecto.

Ahora, si llamar a la función debería cambiar realmente la visibilidad de esas capas, realmente tiene un comportamiento. En ese caso, definitivamente recomendaría métodos separados.

Los parámetros booleanos están bien en el segundo ejemplo. Como ya ha descubierto, los parámetros booleanos no son problemáticos en sí mismos. Es un comportamiento de cambio basado en una bandera, lo cual es problemático.

Sin embargo, el primer ejemplo es problemático, porque la denominación indica un método de establecimiento, pero las implementaciones parecen ser algo diferentes. Así que tienes el antipattern de cambio de comportamiento, y un método con nombre engañoso. Pero si el método en realidad es un configurador regular (sin cambio de comportamiento), entonces no hay problema con setState(boolean) . Teniendo dos métodos, setStateTrue() y setStateFalse() solo complica innecesariamente las cosas sin ningún beneficio.

Otra forma de resolver este problema es introducir un objeto para representar cada sugerencia y hacer que el objeto sea responsable de determinar los valores booleanos asociados con esa sugerencia. De esta manera, puede agregar nuevas permutaciones en lugar de simplemente tener dos estados booleanos.

Por ejemplo, en Java, puedes hacer:

public enum HintState {
    SHOW_HINT(true, false, true),
    HIDE_HINT(false, true, false);

    private HintState(boolean layer1Visible, boolean layer2Visible, boolean layer3Visible) {
         // constructor body and accessors omitted for clarity
    }
}

Y luego su código de llamada se vería así:

setHint(HintState.SHOW_HINT);

Y su código de implementación se vería así:

public void setHint(HintState hint) {
    this.layer1Visible = hint.isLayer1Visible();
    this.layer2Visible = hint.isLayer2Visible();
    this.layer3Visible = hint.isLayer3Visible();
}

Esto mantiene el código de implementación y el código de la persona que llama, a cambio de definir un nuevo tipo de datos que se asigna claramente, con el nombre intenciones a los conjuntos de estados correspondientes. Creo que eso es mejor para todos.

Definiendo la pregunta

Su pregunta del título es "¿Está mal [...]?" - ¿Pero qué quieres decir con "incorrecto"?

Según un compilador de C # o Java, no está mal. Estoy seguro de que eres consciente de eso y no es lo que estás preguntando. Me temo que, aparte de eso, solo tenemos n programmers n+1 opiniones diferentes. Esta respuesta presenta lo que el libro Código limpio tiene que decir al respecto.

Respuesta

Código limpio es un caso sólido contra los argumentos de la función en general:

  

Los argumentos son difíciles. Toman mucho poder conceptual. [...] nuestro   los lectores habrían tenido que interpretarlo cada vez que lo vieron.

Un "lector" aquí puede ser el consumidor de API. También puede ser el siguiente programador, que aún no sabe qué hace este código, lo que podría ser usted en un mes. Pasarán por 2 funciones por separado o por 1 función , una vez teniendo en cuenta true y una vez false .
En resumen, usa la menor cantidad de argumentos posible .

El caso específico de un argumento de indicador se trata más adelante directamente:

  

Los argumentos de la bandera son feos. Pasar un booleano a una función es un verdadero   terrible práctica Inmediatamente complica la firma del   Método, proclamando en voz alta que esta función hace más de una   cosa. Hace una cosa si la bandera es verdadera y otra si la bandera   es falso!

Para responder sus preguntas directamente:
De acuerdo con Clean Code , se recomienda eliminar ese parámetro.

Información adicional:

Su ejemplo es bastante simple, pero incluso allí puede ver la simplicidad que se propaga a su código: las funciones sin parámetros solo hacen asignaciones simples, mientras que la otra función tiene que hacer aritmética booleana para alcanzar el mismo objetivo. Es una aritmética booleana trivial en este ejemplo simplificado, pero podría ser bastante compleja en una situación real.

He visto muchos argumentos aquí que debes hacer que dependan del usuario de la API, porque tener que hacer esto en muchos lugares sería una estupidez:

if (isAfterSunset) light.TurnOn();
else light.TurnOff();

Yo hago estoy de acuerdo en que algo no óptimo está sucediendo aquí. Tal vez sea demasiado obvio para verlo, pero su primera oración menciona "la importancia de evitar [sic] el uso de parámetros booleanos para determinar un comportamiento" y esa es la base de toda la pregunta. No veo una razón para hacer que lo que es malo sea más fácil para el usuario de la API.

No sé si realiza pruebas; en ese caso, también considere esto:

  

Los argumentos son aún más difíciles desde el punto de vista de las pruebas. Imagina el   Dificultad de escribir todos los casos de prueba para asegurar que todos los   Varias combinaciones de argumentos funcionan correctamente. Si no hay   Argumentos, esto es trivial.

  

Entonces, mi pregunta es, si el parámetro booleano se usa para determinar solo los valores, pero no los comportamientos (por ejemplo: no if-else en ese parámetro), ¿todavía se recomienda eliminar ese parámetro booleano?

Cuando tengo dudas sobre tales cosas. Me gusta ver cómo se vería la traza de la pila.

Durante muchos años trabajé en un proyecto de PHP que usaba la misma función que un setter y getter . Si pasa nulo , devolverá el valor; de lo contrario, configúrelo. Fue horrible trabajar con .

Aquí hay un ejemplo de un seguimiento de pila:

function visible() : line 440
function parent() : line 398
function mode() : line 384
function run() : line 5

No tenías idea del estado interno y eso hizo que la depuración fuera más difícil. Hay un montón de otros efectos secundarios negativos, pero trate de ver que hay valor en un detallado nombre de función y claridad cuando las funciones realizan un < em> single action.

Ahora la imagen funciona con el seguimiento de pila para una función que tiene un comportamiento A o B basado en un valor booleano.

function bar() : line 92
function setVisible() : line 120
function foo() : line 492
function setVisible() : line 120
function run() : line 5

Eso es confuso si me preguntas. Las mismas líneas setVisible producen dos rutas de seguimiento diferentes.

Entonces volvamos a tu pregunta. Intente imaginar cómo se verá el seguimiento de la pila, cómo se comunica a una persona lo que está sucediendo y pregúntese si está ayudando a una futura persona a depurar el código.

Aquí hay algunos consejos:

• un nombre de función claro que implica intención sin necesidad de conocer los valores de los argumentos.
• una función realiza una sola acción
• el nombre implica mutación o comportamiento inmutable
• resuelva los desafíos de depuración relacionados con la capacidad de depuración del lenguaje y las herramientas.

A veces, el código parece excesivo con un microscopio, pero cuando retrocedes a la imagen general, un enfoque minimalista lo hará desaparecer. Si necesitas sobresalir para que puedas mantenerlo. Agregar muchas funciones pequeñas puede parecer demasiado detallado, pero mejora la capacidad de mantenimiento cuando se usa ampliamente en un contexto más amplio.

En casi todos los casos en los que está pasando un parámetro boolean a un método como marca para cambiar el comportamiento de algo, debe considerar un explícito y Forma segura de hacer esto.

Si no hace nada más que usar un Enum que representa el estado en que ha mejorado la comprensión de su código.

Este ejemplo utiliza la clase Node de JavaFX :

public enum Visiblity
{
    SHOW, HIDE

    public boolean toggleVisibility(@Nonnull final Node node) {
        node.setVisible(!node.isVisible());
    }
}

siempre es mejor que, como se encuentra en muchos objetos JavaFX :

public void setVisiblity(final boolean flag);

pero creo que .setVisible() y .setHidden() son la mejor solución para la situación en la que flag es un boolean , ya que es la más explícita y menos detallada.

en el caso de algo con selecciones múltiples, es aún más importante hacerlo de esta manera. EnumSet existe solo por este motivo.

  

Ed Mann tiene una muy buena publicación en el blog sobre este tema. yo estaba a punto   Parafraseando justo lo que él dice, para no duplicar el esfuerzo, lo haré.   solo publique un enlace a su blog post como un anexo a esta respuesta.

Al decidir entre un enfoque para alguna interfaz que pasa un parámetro (booleano) frente a métodos sobrecargados sin dicho parámetro, mire a los clientes consumidores.

Si todos los usos pasarían valores constantes (por ejemplo, verdaderos, falsos), entonces eso es un argumento para las sobrecargas.

Si todos los usos pasarían un valor variable, entonces se argumenta el método con enfoque de parámetros.

Si ninguno de esos extremos se aplica, eso significa que hay una combinación de usos del cliente, por lo que tiene que elegir si respaldar ambas formas o hacer que una categoría de clientes se adapte a la otra (lo que para ellos es una opción más antinatural). ) estilo.

Hay dos consideraciones para diseñar:

• API: qué interfaz presentas al usuario,
• Implementación: claridad, mantenimiento, etc ...

No deben estar combinados.

Está perfectamente bien:

• tiene varios métodos en el delegado de la API para una sola implementación,
• tenga un solo método en el envío de API a varias implementaciones según la condición.

Como tal, cualquier argumento que intente equilibrar los costos / beneficios de un diseño de API por los costos / beneficios de un diseño de implementación es dudoso y se debe examinar cuidadosamente.

En el lado de la API

Como programador, generalmente prefiero las API programables. El código es mucho más claro cuando puedo enviar un valor que cuando necesito una declaración if / switch sobre el valor para decidir a qué función llamar.

Esto último puede ser necesario si cada función espera diferentes argumentos.

En su caso, por lo tanto, un solo método setState(type value) parece mejor.

Sin embargo , no hay nada peor que% sin nombre true , false , 2 , etc ... esos valores mágicos no tienen significado por sí mismos. Evita obsesión primitiva , y abraza la escritura fuerte.

Por lo tanto, desde una API POV, quiero: setState(State state) .

En el lado de la implementación

Recomiendo hacer lo que sea más fácil.

Si el método es simple, es mejor mantenerlo juntos. Si el flujo de control es complicado, lo mejor es separarlo en varios métodos, cada uno de los cuales se ocupa de un subcaso o un paso de la tubería.

Finalmente, considera agrupar .

En su ejemplo (con espacios en blanco agregados para facilitar la lectura):

this.layer1.visible = isHintOn;
this.layer2.visible = ! isHintOn;
this.layer3.visible = isHintOn;

¿Por qué layer2 se opone a la tendencia? ¿Es una característica o es un error?

Podría ser posible tener 2 listas [layer1, layer3] y [layer2] , con un nombre explícito que indique la razón por la que se agrupan, y luego iterar sobre esas listas.

Por ejemplo:

for (auto layer : this.mainLayers) { // layer2
    layer.visible = ! isHintOn;
}
for (auto layer : this.hintLayers) { // layer1 and layer3
    layer.visible = isHintOn;
}

El código habla por sí mismo, está claro por qué hay dos grupos y su comportamiento difiere.

Aparte de la pregunta setOn() + setOff() vs set(flag) , consideraría cuidadosamente si un tipo booleano es mejor aquí. ¿Estás seguro de que nunca habrá una tercera opción?

Podría valer la pena considerar una enumeración en lugar de un booleano. Además de permitir la extensibilidad, esto también hace que sea más difícil hacer que el booleano se desarrolle de manera incorrecta, por ejemplo:

setHint(false)

vs

setHint(Visibility::HIDE)

Con la enumeración, será mucho más fácil extender cuando alguien decida que quiere una opción 'si es necesario':

enum class Visibility {
  SHOW,
  HIDE,
  IF_NEEDED // New
}

vs

setHint(false)
setHint(true)
setHintAutomaticMode(true) // New

  

Según ..., sé la importancia de evitar el uso de parámetros booleanos para determinar un comportamiento

Sugeriría reevaluar este conocimiento.

En primer lugar, no veo la conclusión que está proponiendo en la pregunta de SE que vinculó. La mayoría de ellos hablan de reenviar un parámetro en varios pasos de llamadas a métodos, donde se evalúa muy lejos en la cadena.

En su ejemplo, está evaluando el parámetro directamente en su método. A este respecto, no se diferencia en absoluto de ningún otro tipo de parámetro.

En general, no hay absolutamente nada de malo en usar parámetros booleanos; y, obviamente, cualquier parámetro determinará un comportamiento, o ¿por qué lo tendría en primer lugar?