Clean Code - Devo alterar o literal 1 para uma constante?

14

Para evitar números mágicos, muitas vezes ouvimos dizer que devemos dar um nome literal e significativo. Tais como:

//THIS CODE COMES FROM THE CLEAN CODE BOOK
for (int j = 0; j < 34; j++) {
    s += (t[j] * 4) / 5;
}

-------------------- Change to --------------------

int realDaysPerIdealDay = 4;
const int WORK_DAYS_PER_WEEK = 5;
int sum = 0;
for (int j = 0; j < NUMBER_OF_TASKS; j++) {
    int realTaskDays = taskEstimate[j] * realDaysPerIdealDay;
    int realTaskWeeks = (realdays / WORK_DAYS_PER_WEEK);
    sum += realTaskWeeks;
}

Eu tenho um método fictício como este:

Explain: I suppose that I have a list people to serve and by default, we spend $5 to buy food only, but when we have more than one person, we need to buy water and food, we must spend more money, maybe $6. I'll change my code, please focus on the literal 1, my question about it.

public int getMoneyByPersons(){
    if(persons.size() == 1){ 
        // TODO - return money for one person
    } else {
        // TODO - calculate and return money for people.
    }

}

Quando pedi a meus amigos que revisassem meu código, um deles disse que dar um nome ao valor 1 produziria um código mais limpo e o outro dizia que não precisávamos de um nome constante aqui porque o valor é significativo por si só.

Então, minha pergunta é Devo dar um nome para o valor literal 1? Quando é um valor um número mágico e quando não é? Como posso distinguir contexto para escolher a melhor solução?

    
por Jacky 11.09.2018 / 05:16
fonte

6 respostas

23

Não. Nesse exemplo, 1 é perfeitamente significativo.

No entanto, e se persons.size () for zero? Parece estranho que persons.getMoney() funcione para 0 e 2, mas não para 1.

    
por 11.09.2018 / 07:20
fonte
19

Por que um trecho de código contém esse valor literal específico?

  • Esse valor tem um significado especial no domínio do problema ?
  • Ou esse valor é apenas um detalhe de implementação , em que esse valor é uma conseqüência direta do código circundante?

Se o valor literal tiver um significado que não esteja claro no contexto, então sim, dar um nome a esse valor por meio de uma constante ou variável é útil. Mais tarde, quando o contexto original for esquecido, o código com nomes de variáveis significativos será mais sustentável. Lembre-se de que o público-alvo do seu código não é basicamente o compilador (o compilador terá prazer em trabalhar com código horrível), mas futuros mantenedores desse código - que apreciarão se o código for um pouco autoexplicativo.

  • No seu primeiro exemplo, o significado de literais como 34 , 4 , 5 não é aparente no contexto. Em vez disso, alguns desses valores têm um significado especial no domínio do seu problema. Por isso, era bom dar-lhes nomes.

  • No seu segundo exemplo, o significado do literal 1 é muito claro no contexto. Introduzir um nome não é útil.

Na verdade, a introdução de nomes para valores óbvios também pode ser ruim, pois oculta o valor real.

  • Isso pode obscurecer bugs se o valor nomeado for alterado ou estiver incorreto, especialmente se a mesma variável for reutilizada em trechos de código não relacionados.

  • Um pedaço de código também pode funcionar bem para um valor específico, mas pode estar incorreto no caso geral. Ao introduzir abstração desnecessária, o código não é mais obviamente correto.

Não há limite de tamanho em literais “óbvios” porque isso depende totalmente do contexto. Por exemplo. o literal 1024 pode ser totalmente óbvio no contexto do cálculo do tamanho do arquivo, ou o literal 31 no contexto de uma função hash, ou o literal padding: 0.5em no contexto de uma folha de estilo CSS.

    
por 11.09.2018 / 13:29
fonte
8

Existem vários problemas com este pedaço de código, que, a propósito, pode ser encurtado assim:

public List<Money> getMoneyByPersons() {
    return persons.size() == 1 ?
        moneyService.getMoneyIfHasOnePerson() :
        moneyService.getMoney(); 
}
  1. Não está claro por que uma pessoa é um caso especial. Suponho que exista uma regra comercial específica que diga que obter dinheiro de uma pessoa é radicalmente diferente de obter dinheiro de várias pessoas. No entanto, eu tenho que ir olhar dentro de ambos getMoneyIfHasOnePerson e getMoney , na esperança de entender por que existem casos distintos.

  2. O nome getMoneyIfHasOnePerson não parece correto. A partir do nome, eu esperaria que o método verificasse se havia uma única pessoa e, se esse fosse o caso, recebesse dinheiro dele; Caso contrário, não faça nada. De seu código, isso não é o que está acontecendo (ou você está fazendo a condição duas vezes).

  3. Existe algum motivo para devolver um List<Money> em vez de uma coleção?

Voltando à sua pergunta, porque não está claro por que há um tratamento especial para uma pessoa, o dígito deve ser substituído por uma constante, a menos que haja outra maneira de tornar as regras explícitas. Aqui, um não é muito diferente de qualquer outro número mágico. Você pode ter regras comerciais informando que o tratamento especial se aplica a uma, duas ou três pessoas, ou apenas a mais de doze pessoas.

How can I distinguish context to choose the best solution?

Você faz o que torna seu código mais explícito.

Exemplo 1

Imagine o seguinte código:

if (sequence.size() == 0) {
    return null;
}

return this.processSequence(sequence);

O zero é um valor mágico? O código é bastante claro: se não houver elementos na sequência, não vamos processá-lo e retornar um valor especial. Mas esse código também pode ser reescrito assim:

if (sequence.isEmpty()) {
    return null;
}

return this.processSequence(sequence);

Aqui, não mais constante, e o código é ainda mais claro.

Exemplo 2

Pegue outro trecho de código:

const result = Math.round(input * 1000) / 1000;

Não é preciso muito tempo para entender o que isso faz em linguagens como JavaScript, que não têm round(value, precision) overload.

Agora, se você quiser introduzir uma constante, como ela seria chamada? O termo mais próximo que você pode obter é Precision . Então:

const precision = 1000;
const result = Math.round(input * precision) / precision;

Melhora a legibilidade? Pode ser. Aqui, o valor de uma constante é bastante limitado, e você pode se perguntar se realmente precisa realizar a refatoração. A coisa boa aqui é que agora, a precisão é declarada apenas uma vez, então se ela mudar, você não corre o risco de cometer um erro como:

const result = Math.round(input * 100) / 1000;

alterando o valor em um local e esquecendo de fazê-lo no outro.

Exemplo 3

A partir desses exemplos, você pode ter a impressão de que os números devem ser substituídos por constantes em todos os casos . Isso não é verdade. Em algumas situações, ter uma constante não leva à melhoria do código.

Pegue o seguinte trecho de código:

class Point
{
    ...
    public void Reset()
    {
        x, y = (0, 0);
    }
}

Se você tentar substituir zeros por uma variável, a dificuldade seria encontrar um nome significativo. Como você nomearia isso? %código%? %código%? %código%? A introdução de uma constante aqui não aumentaria o código de nenhuma maneira. Isso levaria um pouco mais de tempo e apenas isso.

Tais casos são, no entanto, raros. Então, sempre que você encontrar um número no código, faça um esforço para descobrir como o código pode ser refatorado. Pergunte a si mesmo se existe um significado comercial para o número. Se sim, uma constante é obrigatória. Se não, como você nomearia o número? Se você encontrar um nome significativo, ótimo. Se não, é provável que você tenha encontrado um caso em que a constante é desnecessária.

    
por 11.09.2018 / 13:35
fonte
3

Você pode criar uma função que use um único parâmetro e retorne quatro vezes dividida por cinco que ofereça um alias limpo para o que ele faz enquanto usa o primeiro exemplo.

Estou apenas oferecendo minha própria estratégia habitual, mas talvez aprenda algo também.

O que eu estou pensando é.

// Just an example name  
function normalize_task_value(task) {  
    return (task * 4) / 5;  // or to 'return (task * 4) / NUMBER_OF_TASKS' but it really matters what logic you want to convey and there are reasons to many ways to make this logical. A comment would be the greatest improvement

}  

// Is it possible to just use tasks.length or something like that?  
// this NUMBER_OF_TASKS is the only one thats actually tricky to   
// understand right now how it plays its role.  

function normalize_all_task_values(tasks, accumulator) {  
    for (int i = 0; i < NUMBER_OF_TASKS; i++) {  
        accumulator += normalize_task_value(tasks[i]);
    }
}  

Desculpe se estou fora da base, mas sou apenas um desenvolvedor de javascript. Eu não tenho certeza de qual composição eu justifiquei, acho que nem tudo tem que estar em uma matriz ou lista, mas o comprimento faria muito sentido. E o * 4 faria a boa constante desde que sua origem é nebulosa.

    
por 11.09.2018 / 06:26
fonte
2

Esse número 1, poderia ser um número diferente? Poderia ser 2, ou 3, ou há razões lógicas por que deve ser 1? Se deve ser 1, então usar 1 é bom. Caso contrário, você pode definir uma constante. Não chame isso de constante. (Eu vi isso feito).

60 segundos em um minuto - você precisa de uma constante? Bem, é 60 segundos, não 50 ou 70. E todo mundo sabe disso. Então isso pode ficar um número.

60 itens impressos por página - esse número pode ter sido facilmente 59 ou 55 ou 70. Na verdade, se você alterar o tamanho da fonte, pode se tornar 55 ou 70. Portanto, uma constante significativa é mais solicitada.

É também uma questão de quão claro é o significado. Se você escrever "minutos = segundos / 60", está claro. Se você escrever "x = y / 60", isso não está claro. Deve haver alguns nomes significativos em algum lugar.

Existe uma regra absoluta: não há regras absolutas. Com prática você descobrirá quando usar números e quando usar constantes nomeadas. Não faça isso porque um livro diz isso - até que você entenda porque diz isso.

    
por 12.09.2018 / 01:18
fonte
0

Eu vi uma quantidade razoável de código como no OP (modificado) em recuperações de banco de dados. A consulta retorna uma lista, mas as regras de negócios dizem que pode haver apenas um elemento. E então, é claro, algo mudou para 'apenas este caso' para uma lista com mais de um item. (Sim, eu disse muitas vezes. É quase como se ... nm)

Então, em vez de criar uma constante, eu criaria um método (no método de código limpo) para fornecer um nome ou deixar claro o que a condição deve detectar (e encapsular como ele é detectado):

public int getMoneyByPersons(){
  if(isSingleDepartmentHead()){ 
    // TODO - return money for one person
  } else {
    // TODO - calculate and return money for people.
  }
}

public boolean isSingleDepartmentHead() {
   return persons.size() == 1;
}
    
por 21.11.2018 / 12:46
fonte