Comentário correto para argumentos de função booleana que são "falsos"?

19

Em alguns projetos de código aberto, reuni o seguinte estilo de codificação

void someFunction(bool forget);

void ourFunction() {
  someFunction(false /* forget */);
}    

Eu sempre tenho dúvidas sobre o que falsesignifica aqui. Isso significa "esquecer" ou "esquecer" se refere ao parâmetro correspondente (como no caso acima), e "false" significa negá-lo?

Qual estilo é usado com mais frequência e qual é a melhor maneira (ou algumas das melhores maneiras) de evitar a ambiguidade?

Johannes Schaub - litb
fonte
38
usar enums (mesmo se houver apenas 2 opções) em vez de bools
Esailija
21
Alguns idiomas suportam argumentos nomeados. Em tais línguas, você poderia usarsomeFunction(forget: true);
Brian
3
Sinto-me obrigado a fornecer os argumentos de Martin Fowler sobre argumentos de bandeira (também fala sobre levantadores booleanos). Em geral, tente evitá-los.
FGreg 20/09/13
3
Para elaborar o óbvio, os comentários podem mentir. Portanto, é sempre melhor tornar seu código auto-documentado, porque alguns mudarão truepara falsee não atualizarão o comentário. Se você não pode alterar a API, a melhor maneira de comentar isso ésomeFunction( false /* true=forget, false=remember */)
Mark Lakata 20/09/2013
1
@ Bakuriu - na verdade, eu provavelmente ainda teria a API pública com dois métodos separados ( sortAscendinge sortDescending, ou similares). Agora, por dentro , os dois podem chamar o mesmo método privado, que pode ter esse tipo de parâmetro. Na verdade, se o idioma suportado, provavelmente o que eu passar em seria uma função lambda que continha a direcção da ordenação ...
Clockwork-Muse

Respostas:

33

No código de exemplo que você postou, parece forgetser um argumento de flag. (Não posso ter certeza porque a função é puramente hipotética.)

Os argumentos de sinalização são um cheiro de código. Eles indicam que uma função faz mais de uma coisa e uma boa função deve fazer apenas uma coisa.

Para evitar o argumento flag, divida a função em duas funções que explicam a diferença no nome da função.

Argumento de sinalização

serveIceCream(bool lowFat)

Nenhum argumento de sinalização

serveTraditionalIceCream()
serveLowFatIceCream()

edit: Idealmente, você não precisa manter uma função com o parâmetro flag. Existem casos ao longo das linhas do que Fowler chama de implementação emaranhada, em que a separação completa das funções cria código duplicado. No entanto, quanto maior a complexidade ciclomática da função parametrizada, mais forte é o argumento para se livrar dela.


Isso é apenas um palpite, mas um parâmetro chamado forgetparece inveja do recurso. Por que um chamador diz a outro objeto para esquecer algo? Pode haver um problema maior de design.

Aaron Kurtzhals
fonte
4
+17, capacete. Como são os corpos serveTraditionalIceCreame se serveLowFatIceCreamparecem? Eu tenho um enum com 14 tipos de sorvete.
precisa saber é o seguinte
13
Para métodos públicos, essa convenção é boa, mas como JohnMark alude, a outra metade do SRP é "um bom método deve ser o único método que faz o que faz". Vejo que existem variantes N + 1 desse método; N público sem parâmetros, todos os quais chamam um método privado com o parâmetro que você está tentando evitar expor. Em algum momento, você simplesmente desiste e expõe o parâmetro maldito. O cheiro do código não significa necessariamente que o código deve ser refatorado; são apenas coisas que merecem uma segunda olhada em uma revisão de código.
Keith
@ Aaron Por "característica inveja" o que você quis dizer?
21713 Geek
27

Nas palavras dos leigos:

  • false é um literal.
  • você está passando um literal false
  • você está dizendo someFunctionpara não esquecer
  • você está dizendo someFunctionque o parâmetro esquecer éfalse
  • você está dizendo someFunctionpara lembrar

Na minha opinião, seria melhor se a função fosse assim:

void someFunction(bool remember);

o que você pode chamar

void ourFunction() {
  someFunction(true);
} 

ou mantenha o nome antigo, mas altere sua função de wrapper para

void ourFunctionWithRemember() {
  someFunction(false);
} 

EDITAR:

Como a @Vorac afirmou, sempre se esforce para usar palavras positivas. A dupla negação é confusa.

Tulains Córdova
fonte
15
+1 para que a idéia sempre se esforce para usar palavras positivas. A dupla negação é confusa.
Vorac
1
Concordo, ótima ideia. Dizer ao sistema para não fazer algo é confuso. Se estiver aceitando um parâmetro booleano, expresse-o positivamente.
Brandon
Na maioria dos casos, acho que você também desejaria ser mais específico do que remember, a menos que o nome da função tornasse o significado remember muito óbvio. rememberToCleanUp* or *persistou alguma coisa.
itsbruce
14

O parâmetro pode ser bem nomeado; é difícil dizer sem saber o nome da função. Presumo que o comentário foi escrito pelo autor original da função, e foi um lembrete do que passar falseem someFunctionmeio, mas para qualquer um que vem junto depois, é um pouco obscuro à primeira vista.

O uso de um nome de variável positivo (sugerido no Código Completo ) pode ser a alteração mais simples que facilitaria a leitura deste trecho, por exemplo,

void someFunction(boolean remember);

então ourFunctionse torna:

void ourFunction() {
    someFunction(true /* remember */);
}

No entanto, o uso de um enum torna a chamada de função ainda mais fácil de entender, à custa de algum código de suporte:

public enum RememberFoo {
    REMEMBER,
    FORGET
}

...

void someFunction(RememberFoo remember);

...

void ourFunction() {
    someFunction(RememberFoo.REMEMBER);
}

Se você não puder alterar a assinatura someFunctionpor qualquer motivo, o uso de uma variável temporária também facilita a leitura do código, como simplificar condicionais introduzindo uma variável por nenhum outro motivo, além de facilitar a análise do código por humanos .

void someFunction(boolean remember);

...

void ourFunction() {
    boolean remember = false;
    someFunction(remember);
}
Mike Partridge
fonte
1
Definir remembercomo verdadeiro significa esquecer (no seu exemplo de someFunction(true /* forget */);)?
Brian
2
A enumé de longe a melhor solução. Só porque um tipo pode ser representado como - boolou seja, são isomórficos - não significa que deva ser representado como tal. O mesmo argumento se aplica a stringe mesmo int.
Jon Purdy
10

Renomeie a variável para que um valor bool faça sentido.

Isso é um milhão de vezes melhor do que adicionar um comentário para explicar argumentos para uma função porque o nome é ambíguo.

enderland
fonte
3
Isso não responde à pergunta. Tudo é óbvio quando o método é definido e chamado no mesmo arquivo com 4 linhas de distância. Mas e se tudo que você vê no momento é o chamador? E se ele tiver vários booleanos? Às vezes, um simples comentário em linha ajuda bastante.
Brandon
@Brandon Isso não é um argumento contra chamar o booleano doNotPersist (ou, melhor, Persistir). Chamá-lo de "esquecer" sem dizer o que esquecer é francamente inútil. Ah, e um método que usa vários booleanos como uma opção fede ao céu.
itsbruce
5

Crie um booleano local com um nome mais descritivo e atribua o valor a ele. Dessa forma, o significado será mais claro.

void ourFunction() {
    bool takeAction = false;  /* false means to forget */
    someFunction( takeAction );
}    

Se você não pode renomear a variável, o comentário deve ser um pouco mais expressivo:

void ourFunction() {
    /* false means that the method should forget what was requested */
    someFunction( false );
}    

fonte
1
Definitivamente um bom conselho, mas não acho que ele resolva o problema que o /* forget */comentário deve abordar, que é que sem a declaração de função bem na sua frente, pode ser difícil lembrar o que está sendo definido false. (É por isso que eu acho @ conselho de Esailija para adicionar uma enumeração é melhor, e por isso que eu amo línguas que permitem parâmetros nomeados.)
Gort o robô
@StevenBurnap - thanks! Você está certo, pois minha antiga resposta não foi clara o suficiente ao abordar a questão do OP. Eu o editei para torná-lo mais claro.
3

Há um bom artigo que menciona essa situação exata, pois se refere às APIs de estilo Qt. Lá, ele se chama The Boolean Parameter Trap e vale a pena ler.

A essência disso é:

  1. Sobrecarregar a função para que o bool não seja necessário é melhor
  2. Usar um enum (como sugere Esailija) é melhor
Steven Evers
fonte
2

Este é um comentário bizarro.

Do ponto de vista do compilador, someFunction(false /* forget */);é realmente someFunction(false);(o comentário é retirado). Portanto, tudo o que essa linha faz é chamar someFunctioncom o primeiro (e único) argumento definido como false.

/* forget */é apenas o nome do parâmetro Provavelmente, não passa de um lembrete rápido (e sujo), que realmente não precisa estar lá. Basta usar um nome de parâmetro menos ambíguo e você não precisará do comentário.

user2590712
fonte
1

Um dos conselhos do código Clean é minimizar o número de comentários desnecessários 1 (porque eles tendem a apodrecer) e nomear funções e métodos corretamente.

Depois disso, eu apenas removeria o comentário. Afinal, IDEs modernos (como eclipse) abrem uma caixa com código quando você passa o mouse sobre a função. Ver o código deve limpar a ambiguidade.


1 Comentar algum código complexo está ok.

BЈовић
fonte
Quem disse algo assim: "o pior problema do programador é como nomear variável e compensar por um"?
BЈовић
4
Você provavelmente está procurando martinfowler.com/bliki/TwoHardThings.html como a fonte disso. Ouvi dizer que "existem apenas duas coisas difíceis na Ciência da Computação: invalidação de cache, nomeação de coisas e desativação por um erro".
1

Para elaborar o óbvio, os comentários podem mentir. Portanto, é sempre melhor tornar seu código auto-documentado sem recorrer a comentários para explicar, porque alguma pessoa (talvez você) mudará truepara falsee não atualizará o comentário.

Se você não pode alterar a API, recorro a duas opções

  • Altere o comentário para que ele sempre seja verdadeiro, independentemente do código. Se você chamar isso apenas uma vez, esta é uma boa solução, pois mantém a documentação local.
     algumaFunção (falso / * verdadeiro = esquece, falso = lembra * /); `
  • Use # define, especialmente se você chamá-lo mais de uma vez.
     #define FORGET true
     #define REMEMBER false
     someFunction (REMEMBER);
Mark Lakata
fonte
1

Gosto da resposta sobre como tornar o comentário sempre verdadeiro , mas, apesar de bom, acho que ele perde o problema fundamental desse código - ele está sendo chamado com um literal.

Você deve evitar o uso de literais ao chamar métodos. Variáveis ​​locais, parâmetros opcionais, parâmetros nomeados, enumerações - a melhor maneira de evitá-las dependerá do idioma e do que está disponível, mas tente evitá-las. Os literais têm valores, mas não têm significado.

jmoreno
fonte
-1

Em C #, usei parâmetros nomeados para tornar isso mais claro

someFunction(forget: false);

Ou enum:

enum Memory { Remember, Forget };

someFunction(Memory.Forget);

Ou sobrecargas:

someFunctionForget();

Ou polimorfismo:

var foo = new Elephantine();

foo.someFunction();
Jay Bazuzi
fonte
-2

A nomeação deve sempre resolver a ambiguidade dos booleanos. Eu sempre nomeio booleano como 'isThis' ou 'shouldDoThat', por exemplo:

void printTree(Tree tree, bool shouldPrintPretty){ ... }

e assim por diante. Mas quando você está referenciando o código de outra pessoa, é melhor deixar um comentário ao passar valores.

dchhetri
fonte
Como isso responde à pergunta?
gnat
@gnat Eu estava respondendo a sua pergunta sobre como resolver ambiguidade com parâmetros booleanos. Talvez eu tenha lido a pergunta dele errado.
precisa saber é o seguinte