Como envolver uma expressão como função pode ser Código Limpo?

4

Outro programador acabou de começar a trabalhar em nossa equipe e enviou um patch. O que era necessário era ter algo que compara e verificar algumas condições e definir uma propriedade com base no resultado. O patch, em essência, é uma classe que implementa o requisito como algo assim:

class Processor {
    A a;
    B b;

    public function process(A a, B b) {
        this.a = a;
        this.b = b;

        if (false == this.isVerified() && this.equalNames()) {
            this.setVerified();
        }
    }

    private function equalNames(): bool {
        return this.a.name() == this.b.name();
    }

    private function isVerified(): bool {
        return this.a.isVerified();
    }

    private function setVerified() {
        this.a.setVerified(true);
    }
}

O código real provavelmente tinha um pouco mais de detalhes, mas o pseudo código é detalhado o suficiente, eu acho. Basicamente, o que aconteceu quando analisamos o código ficamos estupefatos. Para piorar a situação, ele não poderia dar a explicação além de que seu código é "Código Limpo" e nos dizendo para "Veja como é limpo!" e leia o livro. Na verdade, alguém apontou que, para utilizar seu código, seria necessário instanciá-lo primeiro e, em seguida, chamar process() ao qual sua resposta foi "O que há de errado com isso?"

Como precisávamos fazer o trabalho, um de nós propôs substituir essa classe por um método dentro de AService class:

public function verifyA(A a, B b) {
    if (!a.isVerified() && a.name() == b.name()) {
        a.setVerified(true);
    }
}

É isso que finalmente se fundiu. Agora, eu sinto que quero saber se existe alguma verdade por trás de uma única expressão lógica em uma função porque eu não quero que ele se sinta ignorado (em sua revisão de código ele sempre coloca um comentário sobre as declarações if de todos para colocar eles em função, e até agora ninguém obriga). Também para fazer com que ele ou nós entendamos como explicar o que é o caminho certo.

Eu não li o livro Código Limpo. Eu leio o blog do Tio Bob e entendo colocar lógica de negócios em funções e. Minha pergunta é mais específica para colocar uma única expressão lógica em função e referência ao livro, se houver. A questão secundária é como lidar com esse problema, pois ele surgiu algumas vezes e afetou a dinâmica da equipe.

    
por imel96 19.04.2017 / 15:55
fonte

7 respostas

12

O que constitui "código limpo" sempre depende do contexto. As mesmas linhas de código podem ser perfeitamente limpas ou excessivamente complicadas, dependendo dos requisitos da aplicação.

O design em questão permite que você use a injeção de dependência para substituir as implementações do processo sem afetar os clientes. Portanto, este é um código limpo e sólido se você tiver esse requisito. Se você não tem tal exigência, então é muito complicado e viola os princípios YAGNI e KISS.

Como nem você nem o desenvolvedor puderam explicar o raciocínio do design, acho que a segunda opção é a mais provável. Provavelmente, o desenvolvedor viu esse design em um contexto em que faz sentido, mas o aplicou em um contexto em que não fazia sentido.

    
por 19.04.2017 / 16:40
fonte
5

Você também deve, e talvez em primeiro lugar, olhar a abstração fornecida aos programadores do cliente que usam as classes, e depois, como você está fazendo, olhar a implementação para limpeza, etc.

Se o uso do cliente for assim:

new Process().process(a,b);

A instância Process nem foi capturada para referência futura. Eu diria que esta classe é um exagero e não fornece uma abstração útil (por exemplo, a.verify(b); )

Outro uso potencial é capturar e manter uma instância de Process e usar seu método de processo em diferentes a's e b's:

Process p = new Process();
...
p.process(a,b);
...
p.process(aa,bb);
...

Eu não posso dizer que isso parece uma grande abstração; mas eu vi pior. (Não seria tão ruim se, por algum motivo, a instância Process tomasse alguma configuração.)

No entanto, com relação à implementação, os campos A a; e B b; devem ser removidos em favor da passagem de parâmetros para os métodos privados para tornar a classe apropriada para esse uso (e thread safe como @MrCochese corretamente aponta).

Além disso, nas duas abordagens mostradas, um a é verificado em relação a b e, em seguida, o a é geralmente marcado como verificado, embora tenha sido verificado apenas em relação a um b muito específico. / p>

Aqui está uma abordagem que eu consideraria, que não mostro da implementação, mas do uso do programador cliente, pois considero a abstração fornecida como a principal consideração sobre os detalhes da implementação interna:

Pair p = new Pair(a,b);
...
p.verify();
...

Agora temos uma abstração decente que merece uma classe. É claro que o par pode ser verificado e, uma vez verificado, sabemos o que isso significa: que a foi verificado em relação a b . Também capturaria o estado de verificação na classe Pair em vez de na classe A .

    
por 19.04.2017 / 20:05
fonte
5

É tudo sobre nomeação.

Se você encontrar um nome de função que torne mais claro o significado do resultado da comparação, coloque-o em uma função.

No seu exemplo, este não é o caso. Por exemplo, isVerified() realmente observa exatamente o que está sendo verificado, neste caso

return this.a.isVerified();

mas também pode ser

return this.b.isVerified();

ou

return this.a.isVerified() && this.b.isVerified();
    
por 19.04.2017 / 21:58
fonte
3

A sua pergunta é realmente sobre um forro ou sobre como funciona o seu programador? Você definiu uma tag "Teamwork", então acho que isso pode ser mais o que você está procurando, com base na experiência.

O contexto que você apresentou é muito pequeno para que possamos fazer algo dele. Ele pode estar certo se o código inteiro está se movendo para (ou é) um grande bola de lama . Nesse caso, eu o apoiaria porque ele está tentando ajudar e colocar o aplicativo no caminho certo. Na verdade, poderíamos dizer que ele está realmente se colocando no projeto e quer que ele tenha sucesso! E ele está constantemente apontando falhas no código do outro. Ele realmente quer que as coisas continuem no caminho certo.

Você nos deu um pouco e nos perguntou se esse bit está certo ou errado. Sem saber nada sobre todo o contexto, arquitetura, processo de negócios, infraestrutura, limitações, etc., podemos debater incessantemente e, principalmente, sobre as preferências de alguém.

Por outro lado, se você tem uma arquitetura strong e boa que funciona e é comprovada ao longo de muitos anos e extensões e outros requisitos de negócios que são colocados em software e infraestrutura e é econômica, ele está errado porque não está trabalhando como A equipe é irrelevante se seu código estiver limpo ou não, incluindo o forro. Mas isso também é subjetivo, porque a maioria das pessoas percebe seu código como o melhor. Ele talvez veja algo que você não vê. Talvez você devesse deixá-lo falar sobre o assunto para toda a equipe como um debate?

    
por 19.04.2017 / 22:48
fonte
2

Existe uma escola de pensamento que pensa assim. Existem muitas outras escolas que acham que é excessivo. Pessoalmente, acho que para simples comparações como essa é um BS total e realmente prejudicial (especialmente, é um sério impacto no desempenho se a comparação é feita com frequência).

A "lógica" é algo como "se pode ser chamada mais de uma vez, faça disso uma função". E uma comparação lógica de curso é chamada mais de uma vez em um trecho de código.

O pior que eu vi foi uma parte do código C onde o programador inicial tinha feito uma função

int add(int a, int b) {
  return a+b;
}

e essa função foi chamada milhões de vezes na execução do programa, fazendo com que ele diminuísse para um rastreamento por causa da enorme quantidade de criação e desempacotamento de pilha de função que aconteceu como resultado.

O programa, que era bastante simples, levou 36 horas para concluir seu trabalho, quando necessário para ser executado uma vez a cada 24 horas. Ao remover essas funções tolas e algum desenrolar de loop, o tempo de execução foi trazido de volta para 16 horas, quando o cliente ficou satisfeito. Poderíamos ter trazido de volta ainda mais, mas isso foi considerado despesa desnecessária.

    
por 19.04.2017 / 16:09
fonte
2

Suponho que você tenha alterado o nome da classe para Processor em nome do exemplo. Mas acho que o nome é realmente fundamental aqui.

Se o desenvolvedor puder criar um nome para a classe que mapeia um conceito de negócio definido , sua abordagem pode fazer sentido, embora eu tenha dúvidas de que a classe é stateful (especialmente se o nome termina com er ). Se for mapeado para um conceito de negócio definido, a classe pode servir como um ponto de extensão e pode melhorar a manutenção.

Se, por outro lado, ele pode ter um nome bobo: um nome que é concatenação de operações (por exemplo, CheckVerifiedAndUpdateProcessor ) ou um nome que é um neologismo arbóreo (por exemplo, CleanNameProcessor ). Se o primeiro, eu perguntaria se o desenvolvedor planeja adicionar uma nova classe em qualquer lugar onde uma expressão composta é usada. Se este último, eu perguntaria se ele espera que todos na equipe mantenham o código para aprender sua pequena terminologia privada. Ambas são ideias terríveis.

    
por 19.04.2017 / 21:44
fonte
2

Algumas vezes, um nome de função fornece melhor abstração do que uma expressão simples. Eu frequentemente me encontro usando:

double square(double in)
{
   return in*in;
}

Uso de

double length_squared = square(a) + square(b) + square(c);

lê melhor que o uso de

double length_squared = a*a + b*b + c*c;

O uso de square faz mais sentido se a , b e c forem obtidos de outras chamadas de função.

double length_squared = square(aFun(x)) + square(bFun(x)) + square(cFun(x));

é definitivamente melhor que:

double a = aFun(x);
double b = bFun(x);
double c = cFun(x);
double length_squared = a*a + b*b + c*c;

Você definitivamente não quer usar:

double length_squared = aFun(x)*aFun(x) + bFun(x)*bFun(x) + cFun(x)*cFun(x);

se aFun , etc., tiver alguma sobrecarga significativa.

    
por 19.04.2017 / 22:49
fonte