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

Explicando Variáveis

Seu caso é um exemplo da introdução da variável explicativa da refatoração. Em suma, 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 legibilidade de desenvolvedor profissional e mantenabilidade são seus objetivos # 1.

Como tal, a regra prática que eu recomendaria é esta: se o propósito do seu parâmetro não for imediatamente óbvio, sinta-se à vontade para usar uma variável para dar um bom nome. Eu acho que isso é 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 ligeiramente 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 realmente destaca por que booleanos em APIs costumam ser uma má ideia - no lado da chamada, eles não fazem nada para explicar o que está acontecendo. Considere:

ParseFolder(true, false);

Você teria que procurar o que esses parâmetros significam; se eles 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 focando na parte dos parâmetros booleanos (para ser justo, era o primeiro parágrafo originalmente). Também acrescentou um exemplo para a primeira parte.

Não escreva códigos desnecessários.

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 oferecer suporte, adicione o nome do parâmetro:

doSomething(preventUndo: true);

Caso contrário, confie no 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ê quiser colocar um ponto de interrupção antes de adicionar um produto ao carrinho para ver se o ID do produto está correto, escrever duas linhas em vez de uma é uma boa ideia.

2. Se você tiver um método com vários 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 é muito complicada. Um exemplo de código que 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 deveria criar variáveis adicionais em casos simples?

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

2. Mas por causa do risco de dissociar o nome que você dá para a variável com o nome do parâmetro.

   Exemplo:

   Digamos que o código original seja:

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

   Mais tarde, um desenvolvedor trabalhando em doSomething percebe que, recentemente, o histórico de desfazer apresentou dois novos métodos:

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

   Agora, preventUndo não parece muito claro. Isso significa que apenas a última ação será evitada? Ou talvez isso signifique que o usuário não poderá usar o recurso de desfazer por mais tempo? Ou que o histórico de desfazer será limpo? Os nomes mais claros seriam:

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

   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 enums?

Algumas outras pessoas sugeriram usar 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. %código%? A sério?! Eu não quero escrever esse if (preventUndo == undoPrevention.prevent) s todas as vezes.

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 método if funcionará como esperado? Para evitar isso, seria necessário escrever esse método 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 booleans começa a parecer tão legal!

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

Nem você deve escrever métodos com sinalizadores booleanos.

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

DoSomething()
{...

DoSomethingUndoable()
{....

Quando você olha para duas classes para ver como elas estão acopladas, uma das categorias é < em> data coupling , que se refere ao código em uma classe chamando métodos de outra classe e apenas transmitindo dados, como em qual mês você quer um relatório, e outra categoria é acoplamento de controle , chamando métodos e passando algo que controla o comportamento do método. O exemplo que eu uso na classe é um sinalizador verbose ou reportType , mas preventUndo também é um ótimo exemplo. Como você acabou de demonstrar, o acoplamento de controle dificulta a leitura do código de chamada e o entendimento do que está acontecendo. Ele também torna o código de chamada vulnerável a alterações em doSomething() que usam o mesmo sinalizador para controlar desfazer e arquivar ou adicionar outro parâmetro a doSomething() para controlar o arquivamento, quebrando assim seu código e assim por diante.

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

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

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

O que eu faria em JavaScript é ter a função pegar um objeto como único parâmetro assim:

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

Eu gosto de aproveitar os recursos de linguagem para ajudar a me esclarecer. 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 options como argumento por esse motivo:

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

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

Isso é apenas a minha preferência embora. Eu suponho que isso vai depender muito do método que está sendo chamado, e 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 de ambas as maneiras e o segundo método é muito pesado e um pouco difícil de entender quando o Undo Undo se expande para páginas e páginas de código, mas faz você pensar. Eu costumo ficar com o método mais simples para substituir um booleano simples por um enum de 2 valores até que eu tenha motivos para mudar. Quando eu adiciono um terceiro valor, eu uso meu IDE para "Find-usages" e corrijo tudo então.

Acho que sua solução torna seu código um pouco mais legível, mas eu evitaria definir uma variável extra apenas para tornar sua 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 for compatível.

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

1. Use um comentário extra

doSomething(/* preventUndo */ true);

2. Use um enum de dois valores em vez de booleano

enum Options
{
    PreventUndo,
    ...
}

...

doSomething(PreventUndo);

Você pode usar a alternativa 2 se seu idioma for compatível com enums.

EDITAR

É claro que usar argumentos nomeados também é uma opção, se o seu idioma oferecer suporte a eles.

Em relação à codificação extra necessária por enums, isso realmente parece insignificante para mim. Em vez de

if (preventUndo)
{
    ...
}

você tem

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

ou

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

E mesmo que seja um pouco mais de digitação, lembre-se de que o código é escrito uma vez e lido muitas vezes, então pode valer a pena digitar um pouco mais agora para encontrar um código mais legível seis meses depois.

Concordo com o que @GlenPeterson disse:

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

mas também insteresting seria o seguinte, pois parece-me que existem apenas duas possibilidades (verdadeiro ou falso)

//Two methods    

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

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

Como você está perguntando sobre o JavaScript principalmente, usando o 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 no link para experimentar as possibilidades

Apenas para uma solução menos convencional e uma vez que o OP mencionou o Javascript, uma possibilidade é usar uma matriz associativa para valores de mapeamento. 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"] ) // ...
}

Eu percebo que isso é um reflexo de alguém de um contexto strongmente 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. Eu não tenho 100% de certeza de que isso está correto. Dê uma olhada em padrões como Factory para 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 que eu concordo. Qualquer diretriz específica, evento "não há argumentos booleanos", deve sempre funcionar como um meio para esse fim e não se tornar e acabar com eles mesmos.

Acho útil observar que esse problema é evitado quando a linguagem de programação suporta argumentos nomeados arguments / keyword, 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 a passagem de 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 em um estágio posterior ou para outros programadores que tendem a ver seu código. Isso também cria um problema ao testar a função. Provavelmente neste caso você tem que criar dois casos de teste. Imagine se você tiver uma função que tenha a instrução switch e altere seu comportamento com base no tipo de comutador e, em seguida, tenha que ter muitos casos de teste diferentes definidos para ela.

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

int preventUndo = true;
doSomething(preventUndo);

Um bom compilador para linguagens low lavel, como C ++, irá otimizar o código de máquina acima de 2 linhas, conforme abaixo:

doSomething(true); 

Por isso, não importa o desempenho, mas aumenta a legibilidade.

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