Definir uma variável para nomear um argumento de método é uma boa prática?

Por questões de legibilidade, frequentemente me pego definindo variáveis ​​temporárias ao chamar funções, como o código a seguir

var preventUndo = true;
doSomething(preventUndo);

A versão mais curta disso para isso seria,

doSomething(true);

Mas quando volto ao código, muitas vezes me pergunto a que truese refere. Existe uma convenção para esse tipo de enigma?

Explicando variáveis

Seu caso é um exemplo da introdução que explica a refatoração de variáveis . Em resumo, uma variável explicativa é aquela que não é estritamente necessária, mas permite que você dê um nome claro a algo, com o objetivo de aumentar a legibilidade.

Código de boa qualidade comunica intenção ao leitor; e como desenvolvedor profissional, a legibilidade e a manutenção são seus objetivos nº 1.

Como tal, a regra geral que eu recomendaria é esta: se o objetivo do seu parâmetro não for imediatamente óbvio, fique à vontade para usar uma variável para lhe dar um bom nome. Eu acho que essa é uma boa prática em geral (a menos que seja abusada). Aqui está um exemplo rápido e artificial - considere:

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

versus o pouco mais longo, mas sem dúvida mais claro:

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

Parâmetros booleanos

Seu exemplo, na verdade, destaca por que os booleanos nas APIs geralmente são uma má idéia - do lado da chamada, eles não fazem nada para explicar o que está acontecendo. Considerar:

ParseFolder(true, false);

Você teria que procurar o que esses parâmetros significam; se fossem enums, seria muito mais claro:

ParseFolder(ParseBehaviour.Recursive, CompatibilityOption.Strict);

Editar:

Adicionamos títulos e trocamos a ordem dos dois parágrafos principais, porque muitas pessoas estavam se concentrando na parte dos parâmetros booleanos (para ser justo, era o primeiro parágrafo originalmente). Também foi adicionado um exemplo à primeira parte.

Não escreva código de que não precisa.

Se você achar doSomething(true)difícil de entender, adicione um comentário:

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

ou, se o idioma suportar, adicione o nome do parâmetro:

doSomething(preventUndo: true);

Caso contrário, conte com o seu IDE para fornecer a assinatura do método chamado:

Casos em que é útil

Colocar uma variável adicional pode ser útil:

1. Para fins de depuração:

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

   O mesmo código pode ser escrito em uma única linha, mas se você deseja colocar um ponto de interrupção antes de adicionar um produto ao carrinho para verificar se o ID do produto está correto, escrever duas linhas em vez de uma é uma boa idéia.

2. Se você tiver um método com muitos parâmetros e cada parâmetro for de uma avaliação. Em uma linha, isso pode se tornar totalmente ilegível.

   // 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. Se a avaliação de um parâmetro for muito complicada. Um exemplo de código que eu já vi:

   // 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 que não em outros casos?

Por que você não deve criar variáveis ​​adicionais em casos simples?

1. Não por causa do impacto no desempenho. Isso seria uma suposição muito errada de um desenvolvedor iniciante que está otimizando micro seu aplicativo. Não há impacto no desempenho na maioria dos idiomas, pois o compilador incorporará a variável. Nos idiomas em que o compilador não faz isso, você pode ganhar alguns microssegundos inserindo-o manualmente, o que não vale a pena. Não faça isso.

2. Mas, devido ao risco de dissociar o nome que você atribui à variável, ao nome do parâmetro.

   Exemplo:

   Digamos que o código original é:

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

   Mais tarde, um desenvolvedor que trabalha com doSomethingavisos de que recentemente o histórico de desfazer introduziu dois novos métodos:

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

   Agora, preventUndonão parece muito claro. Isso significa que apenas a última ação será impedida? Ou talvez isso signifique que o usuário não poderá mais usar o recurso desfazer? Ou que a história de desfazer será limpa? Os nomes mais claros seriam:

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

   Então agora você tem:

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

E as enums?

Algumas outras pessoas sugeriram o uso de enums. Enquanto resolve o problema imediato, cria um maior. Vamos dar uma olhada:

enum undoPrevention
{
    keepInHistory,
    prevent,
}

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

doSomething(undoPrevention.prevent);

Problemas com isso:

1. É muito código. if (preventUndo == undoPrevention.prevent)? A sério?! Eu não quero escrever isso iftoda vez.

2. Adicionar um elemento ao enum é muito tentador depois, se eu estiver usando o mesmo enum em outro lugar. E se eu modificá-lo assim:

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

   O que vai acontecer agora? O doSomethingmétodo funcionará conforme o esperado? Para evitar isso, seria necessário escrever esse método dessa maneira desde o início:

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

   A variante que usa booleanos começa a parecer tão legal!

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

Também não deve escrever métodos com sinalizadores booleanos.

Para citar Robert C. Martin diz em seu livro Clean Code (ISBN-13 978-0-13-235088-4): "Passar um booleano para uma função é uma prática verdadeiramente terrível".

Parafraseando ele, o raciocínio é que o fato de você ter uma opção verdadeira / falsa significa que seu método provavelmente está fazendo duas coisas diferentes (por exemplo, "Faça algo com desfazer" e "Faça algo sem desfazer") e, portanto, deve ser dividido em dois métodos diferentes (que podem chamar internamente a mesma coisa).

DoSomething()
{...

DoSomethingUndoable()
{....

Quando você olha para duas classes para ver como elas são acopladas, uma das categorias é o acoplamento de dados , que se refere ao código em uma classe que chama métodos de outra classe e apenas passa dados, como em que mês você deseja um relatório e outro categoria é o acoplamento de controle , chamando métodos e passando algo que controla o comportamento do método. O exemplo que eu uso na classe é uma verbosebandeira ou uma reportTypebandeira, mas preventUndotambém é um ótimo exemplo. Como você acabou de demonstrar, o acoplamento de controle torna difícil ler o código de chamada e entender o que está acontecendo. Também torna o código de chamada vulnerável a alterações doSomething()que usam o mesmo sinalizador para controlar desfazer e arquivar, ou adicionar outro parâmetro aodoSomething() controlar o arquivamento, quebrando seu código e assim por diante.

O problema é que o código está muito acoplado. Passar um bool para controlar o comportamento é, na minha opinião, um sinal de uma API ruim. Se você possui a API, altere-a. Dois métodos doSomething()e doSomethingWithUndo()seriam melhores. se você não o possui, escreva você mesmo dois métodos de wrapper no código que você possui e faça com que um deles chame doSomething(true)e o outro chame doSomething(false).

Não é papel do chamador definir o papel dos argumentos. Eu sempre uso a versão em linha e verifico a assinatura da função chamada, se tiver alguma dúvida.

A variável deve ser nomeada após sua função no escopo atual. Não O escopo para o qual eles são enviados.

O que eu faria em JavaScript é fazer com que a função pegue um objeto como o único parâmetro parecido com isto:

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

Em seguida, chame-o com:

doSomething({preventUndo: true});

Gosto de aproveitar os recursos da linguagem para ajudar a esclarecer a mim mesmo. Por exemplo, em C #, você pode especificar parâmetros por nome:

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

Em JavaScript, geralmente prefiro usar um objeto de opções como argumento por esse motivo:

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

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

Essa é apenas a minha preferência. Suponho que vai depender muito do método que está sendo chamado e de como o IDE ajuda o desenvolvedor a entender a assinatura.

Acho que é o mais simples e fácil de ler:

enum Undo { ALLOW, PREVENT }

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

doSomething(Undo.ALLOW);

MainMa não gosta disso. Podemos ter que concordar em discordar ou podemos usar uma solução que Joshua Bloch propõe:

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
}

Agora, se você adicionar um Undo.LIMITED_UNDO, seu código não será compilado, a menos que você implemente o método createUndoBuffer (). E em doSomething () não há if (Undo.ALLOW == u). Eu fiz isso de duas maneiras e o segundo método é bastante pesado e um pouco difícil de entender quando a enumeração Undo se expande para páginas e páginas de código, mas isso faz você pensar. Eu normalmente uso o método mais simples de substituir um booleano simples por um enum de 2 valores até que eu tenha motivos para mudar. Quando adiciono um terceiro valor, uso meu IDE para "Encontrar usos" e conserte tudo.

Eu acho que sua solução torna seu código um pouco mais legível, mas evitaria definir uma variável extra apenas para tornar sua chamada de função mais clara. Além disso, se você quiser usar uma variável extra, eu a marcaria como constante se sua linguagem de programação a suportar.

Posso pensar em duas alternativas que não envolvem uma variável extra:

1. Use um comentário extra

doSomething(/* preventUndo */ true);

2. Use uma enumeração de dois valores em vez de booleano

enum Options
{
    PreventUndo,
    ...
}

...

doSomething(PreventUndo);

Você pode usar a alternativa 2 se o seu idioma suportar enumerações.

EDITAR

Obviamente, usar argumentos nomeados também é uma opção, se o seu idioma os suportar.

Em relação à codificação extra necessária para as enumerações, isso realmente me parece insignificante. Ao invés de

if (preventUndo)
{
    ...
}

Você tem

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

ou

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

E mesmo que seja um pouco mais digitado, lembre-se de que o código é escrito uma vez e lido várias vezes, para que valha a pena digitar um pouco mais agora, a fim de encontrar um código mais legível seis meses depois.

Concordo com o que o @GlenPeterson disse:

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

mas também interessante seria o seguinte, pois me parece que há apenas duas possibilidades (verdadeira ou falsa)

//Two methods    

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

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

Como você está perguntando principalmente sobre javascript, usando coffeescript, você pode ter um argumento nomeado como sintaxe, super 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 para

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

Divirta-se em http://js2coffee.org/ para experimentar as possibilidades

Apenas para uma solução menos convencional e, como o OP mencionou o Javascript, é possível usar uma matriz associativa para mapear valores. A vantagem sobre um único booleano é que você pode ter quantos argumentos quiser e não se preocupar com a sequência deles.

var doSomethingOptions = new Array();

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

doSomething( doSomethingOptions );

// ...

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

Sei que isso é um reflexo de alguém de um fundo fortemente tipado e pode não ser tão prático, mas considere isso como originalidade.

Outra possibilidade é ter um objeto e também é possível em Javascript. Não tenho 100% de certeza de que isso esteja sintaticamente correto. Veja padrões como o Factory para obter mais inspiração.

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

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

O foco da sua pergunta e das outras respostas é melhorar a legibilidade onde a função é chamada, que é um foco com o qual concordo. Qualquer orientação específica, evento "sem argumentos booleanos", deve sempre funcionar como um meio para esse fim e não se tornar e terminar por si mesma.

Acho útil notar que esse problema é evitado principalmente quando a linguagem de programação suporta argumentos nomeados / argumentos de palavras-chave, como em C # 4 e Python, ou onde os argumentos do método são intercalados no nome do método, como em Smalltalk ou Objective-C.

Exemplos:

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

# Python
foo.doSomething(preventUndo=True)

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

Deve-se evitar passar variáveis ​​booleanas como argumentos para funções. Porque as funções devem fazer uma coisa de cada vez. Ao passar a variável booleana, a função tem dois comportamentos. Isso também cria um problema de legibilidade para um estágio posterior ou para outros programadores que tendem a ver seu código. Isso também cria problemas ao testar a função. Provavelmente, nesse caso, você deve criar dois casos de teste. Imagine se você tem uma função que possui a instrução switch e altera seu comportamento com base no tipo de switch, então você precisa ter tantos casos de teste diferentes definidos para ela.

Sempre que se depara com um argumento booleano para a função, eles precisam ajustar o código de forma que não escrevam argumento booleano.

int preventUndo = true;
doSomething(preventUndo);

Um bom compilador para linguagens de baixo nível de linguagem, como C ++, otimizará o código da máquina acima de 2 linhas, como abaixo:

doSomething(true); 

portanto, não importa no desempenho, mas aumenta a legibilidade.

Também é útil usar uma variável, caso ela se torne variável mais tarde e possa aceitar outros valores também.