¿Es una buena práctica definir una variable para nombrar un argumento de método?

Explicando variables

Su caso es un ejemplo de la introducir la variable explicativa . En resumen, una variable explicativa es una que no es estrictamente necesaria, pero le permite dar un nombre claro a algo, con el objetivo de aumentar la legibilidad.

El código de buena calidad comunica intención al lector; y como desarrollador profesional, la legibilidad y la capacidad de mantenimiento son sus objetivos principales.

Como tal, la regla de oro que recomendaría es esta: si el propósito de su parámetro no es obvio de inmediato, no dude en usar una variable para darle un buen nombre. Creo que este es un Buenas prácticas en general (salvo abuso). Aquí hay un ejemplo rápido, artificial: considere:

editButton.Enabled = (_grid.SelectedRow != null && ((Person)_grid.SelectedRow).Status == PersonStatus.Active);

frente a un poco más largo, pero posiblemente más claro:

bool personIsSelected = (_grid.SelectedRow != null);
bool selectedPersonIsEditable = (personIsSelected && ((Person)_grid.SelectedRow).Status == PersonStatus.Active)
editButton.Enabled = (personIsSelected && selectedPersonIsEditable);

Parámetros booleanos

Su ejemplo realmente resalta por qué booleans en API son a menudo una mala idea - en el lado de la llamada, no hacen nada para explicar lo que está sucediendo. Considera:

ParseFolder(true, false);

Tendrías que buscar lo que significan esos parámetros; si fueran enumerados, sería mucho más claro:

ParseFolder(ParseBehaviour.Recursive, CompatibilityOption.Strict);

Editar:

Se agregaron encabezados y se cambió el orden de los dos párrafos principales, porque demasiadas personas se estaban enfocando en la parte de parámetros booleanos (para ser justos, era el primer párrafo originalmente). También agregó un ejemplo a la primera parte.

No escriba el código que no necesita.

Si encuentra doSomething(true) difícil de entender, debe agregar un comentario:

// Do something and prevent the undo.
doSomething(true);

o, si el idioma lo admite, agregue el nombre del parámetro:

doSomething(preventUndo: true);

De lo contrario, confíe en su IDE para obtener la firma del método llamado:

Casos en los que es útil

Poner una variable adicional puede ser útil:

1. Para propósitos de depuración:

   var productId = this.Data.GetLastProductId();
   this.Data.AddToCart(productId);
   

   El mismo código puede escribirse en una sola línea, pero si desea colocar un punto de interrupción antes de agregar un producto al carrito para ver si la identificación del producto es correcta, escribir dos líneas en lugar de una es una buena idea.

2. Si tiene un método con muchos parámetros y cada uno de ellos va de una evaluación. En una línea, esto puede volverse totalmente ilegible.

   // Even with indentation, this is unreadable.
   var doSomething(
       isSomethingElse ? 0 : this.getAValue(),
       this.getAnotherOne() ?? this.default,
       (a + b + c + d + e) * f,
       this.hello ? this.world : (this.hello2 ? this.world2 : -1));
   

3. Si la evaluación de un parámetro es demasiado complicada. Un ejemplo de un código que he visto:

   // Wouldn't it be easier to have several if/else's (maybe even in a separate method)?
   do(something ? (hello ? world : -1) : (programmers ? stackexchange : (com ? -1 : 0)));
   

¿Por qué no en otros casos?

¿Por qué no debería crear variables adicionales en casos simples?

1. No por el impacto en el rendimiento. Esta sería una suposición muy errónea de un desarrollador principiante que está micro optimizando su aplicación. No hay impacto en el rendimiento en la mayoría de los idiomas, ya que el compilador incorporará la variable. En aquellos idiomas en los que el compilador no hace eso, puede ganar unos pocos microsegundos al insertarlo a mano, lo que no vale la pena. No hagas eso.

2. Pero debido al riesgo de disociar el nombre que le da a la variable al nombre del parámetro.

   Ejemplo:

   Digamos que el código original es:

   void doSomething(bool preventUndo)
   {
       // Does something very interesting.
       undoHistory.removeLast();
   }
   
   // Later in code:
   var preventUndo = true;
   doSomething(preventUndo);
   

   Más tarde, un desarrollador que trabaja en doSomething advierte que recientemente, el historial de deshacer introdujo dos nuevos métodos:

   undoHistory.clearAll() { ... }
   undoHistory.disable() { ... }
   

   Ahora, preventUndo no parece muy claro. ¿Significa que solo se evitará la última acción? ¿O tal vez significa que el usuario ya no podrá usar la función de deshacer? ¿O que la historia de deshacer se borrará? Los nombres más claros serían:

   doSomething(bool hideLastUndo) { ... }
   doSomething(bool removeAllUndo) { ... }
   doSomething(bool disableUndoFeature) { ... }
   

   Así que ahora tienes:

   void doSomething(bool hideLastUndo)
   {
       // Does something very interesting.
       undoHistory.removeLast();
   }
   
   // Later in code, very error prone, while the signature of the method is clear:
   var preventUndo = true;
   doSomething(preventUndo);
   

¿Qué pasa con las enumeraciones?

Algunas otras personas sugirieron usar enumeraciones. Si bien resuelve el problema inmediato, crea uno más grande. Echemos un vistazo:

enum undoPrevention
{
    keepInHistory,
    prevent,
}

void doSomething(undoPrevention preventUndo)
{
    doTheJob();
    if (preventUndo == undoPrevention.prevent)
    {
        this.undoHistory.discardLastEntry();
    }
}

doSomething(undoPrevention.prevent);

Problemas con eso:

1. Es demasiado código. %código%? ¡¿Seriamente?! No quiero escribir if (preventUndo == undoPrevention.prevent) s cada vez.

2. Agregar un elemento a la enumeración es muy tentador más adelante, si estoy usando la misma enumeración en otro lugar. ¿Qué pasa si lo modifico de esta manera?

   enum undoPrevention
   {
       keepInHistory,
       prevent,
       keepButDisable, // Keeps the entry in the history, but makes it disabled.
   }
   

   ¿Qué pasará ahora? ¿Funcionará el método if como se esperaba? Para evitar esto, sería necesario escribir este método desde el principio:

   void doSomething(undoPrevention preventUndo)
   {
       if (![undoPrevention.keepInHistory, undoPrevention.prevent].contains(preventUndo))
       {
           throw new ArgumentException('preventUndo');
       }
   
       doTheJob();
       if (preventUndo == undoPrevention.prevent)
       {
           this.undoHistory.discardLastEntry();
       }
   }
   

   ¡La variante que usa booleanos comienza a verse muy bien!

   void doSomething(bool preventUndo)
   {
       doTheJob();
       if (preventUndo)
       {
           this.undoHistory.discardLastEntry();
       }
   }
   

Tampoco, no debes escribir métodos con banderas booleanas.

Para citar, Robert C. Martin dice en su libro Clean Code (ISBN-13 978-0-13-235088-4), "Pasar un booleano a una función es una práctica verdaderamente terrible".

Parafraseando a él, el razonamiento es que el hecho de que tengas un cambio verdadero / falso significa que lo más probable es que tu método esté haciendo dos cosas diferentes (es decir, "Hacer algo con deshacer" y "Hacer algo sin deshacer"), y por lo tanto, debe dividirse en dos métodos diferentes (que pueden llamar internamente a la misma cosa).

DoSomething()
{...

DoSomethingUndoable()
{....

Cuando observas dos clases para ver qué tan juntas están, una de las categorías es < em> acoplamiento de datos , que se refiere al código en una clase que llama a los métodos de otra clase y solo pasa datos, como el mes para el que desea un informe, y otra categoría es control de acoplamiento , Llamar a los métodos y pasar algo que controle el comportamiento del método. El ejemplo que uso en clase es un indicador verbose o un indicador reportType , pero preventUndo también es un gran ejemplo. Como acaba de demostrar, el acoplamiento de control hace que sea difícil leer el código de llamada y entender lo que está sucediendo. También hace que el código de llamada sea vulnerable a los cambios en doSomething() que usan la misma marca para controlar tanto deshacer como archivar, o agregar otro parámetro a doSomething() para controlar el archivo, rompiendo así su código, y así sucesivamente.

El problema es que el código está demasiado unido. Pasar un bool para controlar el comportamiento es, en mi opinión, un signo de una API mala. Si tienes la API, cámbiala. Dos métodos, doSomething() y doSomethingWithUndo() , serían mejores. si no lo posee, escriba usted mismo dos métodos de envoltorio en el código que posee, y haga que uno de ellos llame a doSomething(true) y el otro a doSomething(false) .

No es el rol del que llama definir el rol de los argumentos. Siempre busco la versión en línea y verifico la firma de la función llamada si tengo alguna duda.

La variable debe nombrarse después de su rol en el alcance actual. No el alcance donde se envían.

Lo que haría en JavaScript es que la función tome un objeto como único parámetro, como este:

function doSomething(settings) {
    var preventUndo = settings.hasOwnProperty('preventUndo') ? settings.preventUndo : false;
    // Deal with preventUndo in the normal way.
}

Entonces llámalo con:

doSomething({preventUndo: true});

Me gusta aprovechar las características del lenguaje para ayudar a aclararme a mí mismo. Por ejemplo, en C # puede especificar parámetros por nombre:

 CallSomething(name: "So and so", age: 12, description: "A random person.");

En JavaScript, normalmente prefiero usar un objeto de opciones como argumento por este motivo:

function doSomething(args) { /*...*/ }

doSomething({ name: 'So and so', age: 12, description: 'A random person.' });

Eso es solo mi preferencia. Supongo que dependerá en gran medida del método al que se llame y de cómo el IDE ayuda al desarrollador a entender la firma.

Creo que esto es lo más sencillo y fácil de leer:

enum Undo { ALLOW, PREVENT }

doSomething(Undo u) {
    if (Undo.ALLOW == u) {
        // save stuff to undo later.
    }
    // do your thing
}

doSomething(Undo.ALLOW);

A MainMa no le gusta eso. Es posible que tengamos que estar de acuerdo en no estar de acuerdo o podemos usar una solución que Joshua Bloch propone:

enum Undo {
    ALLOW {
        @Override
        public void createUndoBuffer() {
            // put undo code here
        }
    },
    PREVENT {
        @Override
        public void createUndoBuffer() {
            // do nothing
        }
    };

    public abstract void createUndoBuffer();
}

doSomething(Undo u) {
    u.createUndoBuffer();
    // do your thing
}

Ahora, si alguna vez agrega un Undo.LIMITED_UNDO, su código no se compilará a menos que implemente el método createUndoBuffer (). Y en doSomething () no hay if (Undo.ALLOW == u). Lo he hecho de ambas maneras y el segundo método es bastante pesado y un poco difícil de entender cuando la enumeración de Deshacer se expande a páginas y páginas de código, pero te hace pensar. Normalmente me quedo con el método más simple para reemplazar un booleano simple con una enumeración de valor 2 hasta que tenga razones para cambiar. Cuando agrego un tercer valor, uso mi IDE para "Buscar usos" y luego arreglo todo.

Creo que su solución hace que su código sea un poco más legible, pero evitaría definir una variable adicional solo para que su función sea más clara. Además, si desea utilizar una variable adicional, la marcaría como constante si su lenguaje de programación lo admite.

Puedo pensar en dos alternativas que no involucran una variable adicional:

1. Usa un comentario extra

doSomething(/* preventUndo */ true);

2. Utilice una enumeración de dos valores en lugar de booleana

enum Options
{
    PreventUndo,
    ...
}

...

doSomething(PreventUndo);

Puedes usar la alternativa 2 si tu idioma es compatible con enums.

EDIT

Por supuesto, usar argumentos con nombre también es una opción, si su idioma los admite.

En lo que respecta a la codificación adicional que necesitan las enumeraciones, realmente me parece despreciable. En lugar de

if (preventUndo)
{
    ...
}

tienes

if (undoOption == PreventUndo)
{
    ...
}

o

switch (undoOption)
{
  case PreventUndo:
  ...
}

E incluso si es un poco más de escritura, recuerde que el código se escribe una vez y se lee muchas veces, por lo que puede valer la pena escribir un poco más ahora para encontrar un código más legible seis meses después.

Estoy de acuerdo con lo que dijo @GlenPeterson:

doSomething(Undo.ALLOW); // call using a self evident enum

pero también sería interesante lo siguiente, ya que me parece que solo hay dos posibilidades (verdadera o falsa)

//Two methods    

doSomething()  // this method does something but doesn't prevent undo

doSomethingPreventUndo() // this method does something and prevents undo

Ya que estás preguntando sobre javascript principalmente, al usar coffeescript puedes tener un argumento con nombre como sintaxis, muy fácil:

# declare method, ={} declares a default value to prevent null reference errors
doSomething = ({preventUndo} = {}) ->
  if preventUndo
    undo = no
  else
    undo = yes

#call method
doSomething preventUndo: yes
#or 
doSomething(preventUndo: yes)

compila a

var doSomething;

doSomething = function(_arg) {
  var preventUndo, undo;
  preventUndo = (_arg != null ? _arg : {}).preventUndo;
  if (preventUndo) {
    return undo = false;
  } else {
    return undo = true;
  }
};

doSomething({
  preventUndo: true
});

doSomething({
  preventUndo: true
});

Diviértase un poco en enlace para probar las posibilidades

Solo para una solución menos convencional y, como el OP mencionó Javascript, una posibilidad es usar una matriz asociativa para asignar valores. La ventaja sobre un solo booleano es que puede tener tantos argumentos como desee y no preocuparse por su secuencia.

var doSomethingOptions = new Array();

doSomethingOptions["PREVENTUNDO"] = true;
doSomethingOptions["TESTMODE"] = false;

doSomething( doSomethingOptions );

// ...

function doSomething( doSomethingOptions ){
    // Check for null here
    if( doSomethingOptions["PREVENTUNDO"] ) // ...
}

Me doy cuenta de que esto es un reflejo de alguien con antecedentes muy marcados y puede que no sea tan práctico, pero considera esto por su originalidad.

Otra posibilidad es tener un objeto y también es posible en Javascript. No estoy 100% seguro de que esto sea sintácticamente correcto. Eche un vistazo a patrones como Factory para más inspiración.

function SomethingDoer( preventUndo ) {
    this.preventUndo = preventUndo;
    this.doSomething = function() {
            if( this.preventUndo ){
                    // ...
            }
    };
}

mySomethingDoer = new SomethingDoer(true).doSomething();

El enfoque de su pregunta y de las otras respuestas es mejorar la legibilidad donde se llama la función, que es un enfoque con el que estoy de acuerdo. Cualquier guía específica, evento "sin argumentos booleanos", debería funcionar siempre como medio para este fin y no convertirse y terminar en sí mismos.

Creo que es útil tener en cuenta que este problema se evita principalmente cuando el lenguaje de programación admite argumentos con nombre / palabras clave, como en C # 4 y Python, o cuando los argumentos del método están intercalados en el nombre del método, como en Smalltalk o en Objective -C.

Ejemplos:

// C# 4
foo.doSomething(preventUndo: true);

# Python
foo.doSomething(preventUndo=True)

// Objective-C
[foo doSomethingWith:bar shouldPreventUndo:YES];

Uno debe evitar pasar variables booleanas como argumentos a funciones. Porque las funciones deberían hacer una cosa a la vez. Al pasar la variable booleana la función tiene dos comportamientos. Esto también crea problemas de legibilidad para una etapa posterior o para otros programadores que tienden a ver su código. Esto también crea problemas al probar la función. Probablemente en este caso tengas que crear dos casos de prueba. Imagine que si tiene una función que tiene una instrucción de cambio y cambia su comportamiento en función del tipo de cambio, entonces tiene que tener definidos muchos casos de prueba diferentes.

Cuando uno se encuentra con un argumento booleano para la función, tiene que modificar el código para que no escriba ningún argumento booleano.

int preventUndo = true;
doSomething(preventUndo);

Un buen compilador para lenguajes de bajo nivel como C ++ optimizará el código de máquina de la línea anterior 2 como se muestra a continuación:

doSomething(true); 

por lo que no importa en el rendimiento, pero aumentará la legibilidad.

También es útil usar una variable en caso de que se convierta en una variable más adelante y también pueda aceptar otros valores.