Código limpo: consequências de métodos curtos com poucos parâmetros

15

Recentemente, durante uma revisão de código, me deparei com um código escrito por um novo colega, que contém um padrão com um cheiro. Suspeito que as decisões de meu colega sejam baseadas em regras propostas pelo famoso livro Clean Code (e talvez por outros livros semelhantes).

Entendo que o construtor da classe é totalmente responsável pela criação de um objeto válido e que sua principal tarefa é a atribuição das propriedades (privadas) de um objeto. Obviamente, pode ocorrer que valores de propriedades opcionais possam ser definidos por outros métodos que não o construtor da classe, mas essas situações são bastante raras (embora não necessariamente erradas, desde que o restante da classe leve em consideração a opcionalidade dessa propriedade). Isso é importante, pois permite garantir que o objeto esteja sempre em um estado válido.

No entanto, no código que encontrei, a maioria dos valores de propriedades é realmente definida por outros métodos que não o construtor. Os valores resultantes dos cálculos são atribuídos a propriedades a serem usadas dentro de vários métodos particulares em toda a classe. Aparentemente, o autor usa propriedades de classe como se fossem variáveis ​​globais que deveriam estar acessíveis em toda a classe, em vez de parametrizar esses valores para as funções que precisam delas. Além disso, os métodos da classe devem ser chamados em uma ordem específica, porque a classe não fará muito de outra maneira.

Suspeito que esse código tenha sido inspirado pelo conselho de manter os métodos curtos (<= 5 linhas de código), para evitar grandes listas de parâmetros (<3 parâmetros) e que os construtores não devem funcionar (como executar algum tipo de cálculo) essencial para a validade do objeto).

Agora, é claro, eu poderia argumentar contra esse padrão se puder provar que todos os tipos de erros indefinidos potencialmente surgem quando os métodos não são chamados em uma ordem específica. No entanto, prevejo que a resposta a isso incluirá validações que verificam que as propriedades devem ser configuradas quando os métodos são chamados e que precisam dessas propriedades.

No entanto, prefiro alterar completamente o código, para que a classe se torne uma cópia azul de um objeto real, em vez de uma série de métodos que devem ser chamados (proceduralmente) em uma ordem específica.

Eu sinto que o código que encontrei cheira. Na verdade, acredito que exista uma distinção bastante clara sobre quando salvar um valor em uma propriedade de classe e quando colocá-lo em um parâmetro para um método diferente de usar - eu realmente não acredito que eles possam ser alternativas um para o outro . Estou procurando as palavras para esta distinção.

user2180613
fonte
6
1. Brincando de advogado do diabo por um momento ... O código realmente funciona? Por causa de Transferência de Dados Objects são uma técnica perfeitamente válida, e se isso é tudo isso é ...
Robert Harvey
7
2. Se você não tiver as palavras para descrever o problema, não terá experiência suficiente para refutar a posição do seu colega.
Robert Harvey
4
3. Se você tiver algum código de trabalho, poderá publicá-lo na Revisão do Código e deixe-os dar uma olhada. Caso contrário, isso é apenas uma generalidade errante.
Robert Harvey
5
@RobertHarvey "Os valores que resultam dos cálculos são atribuídos a propriedades a serem usadas dentro de vários métodos particulares em toda a classe" não me parece um DTO que se preze. Concordo que um pouco mais de especificidade seria útil.
topo Reinstate Monica
4
Aparte: Parece que alguém realmente não leu o Código Limpo antes de respondê-lo. Acabei de digitalizá-lo novamente e não consegui encontrar nenhum lugar onde sugerisse que "construtores não deveriam funcionar" (alguns exemplos de fato realizam trabalho), e a solução sugerida para evitar muitos parâmetros é criar um objeto de parâmetro que consolide grupos de parâmetros, não bastardize suas funções. E o livro não sugerem refatoração de código para evitar dependências temporais entre métodos. Acho que seu viés contra alguns dos estilos de código preferidos dele coloriu sua percepção do livro.
Eric King

Respostas:

13

Como alguém que leu o Código Limpo e assistiu à série Código Limpo várias vezes e muitas vezes ensina e treina outras pessoas a escrever código mais limpo, posso realmente garantir que suas observações estão corretas - as métricas que você aponta são todas mencionadas no livro .

No entanto, o livro prossegue com outros pontos que também devem ser aplicados juntamente com as diretrizes que você apontou. Eles foram aparentemente ignorados no código com o qual você está lidando. Isso pode ter acontecido porque seu colega ainda está na fase de aprendizado; nesse caso, quanto for necessário apontar o cheiro do código, é bom lembrar que ele está fazendo isso de boa vontade, aprendendo e tentando para escrever um código melhor.

O Código Limpo propõe que os métodos sejam curtos, com o mínimo de argumentos possível. Mas, seguindo essas diretrizes, propõe que devemos seguir os princípios do S OLID, aumentar a coesão e reduzir o acoplamento .

O S em povoamentos sólida para Individual de Responsabilidade Princípio, que afirma que um objeto deve ser responsável por apenas uma coisa. "Coisa" não é um termo muito preciso, portanto as descrições desse princípio variam muito. No entanto, o tio Bob, autor do Código Limpo, também é a pessoa que cunhou esse princípio, descrevendo-o como: "Reúna as coisas que mudam pelas mesmas razões. Separe as coisas que mudam por diferentes razões". Ele continua dizendo o que ele quer dizer com razões para mudar aqui e aqui(uma explicação mais longa aqui seria demais). Se esse princípio foi aplicado à classe com a qual você está lidando, é muito provável que as partes que lidam com os cálculos sejam separadas das que lidam com o estado de espera, dividindo a classe em duas ou mais, dependendo de quantas razões para mudar esses cálculos.

Além disso, as classes Clean devem ser coesas , o que significa que a maioria de seus métodos usa a maioria de seus atributos. Como tal, uma classe maximamente coesa é aquela em que todos os métodos usam todo o seu atributo; como exemplo, em um aplicativo gráfico, você pode ter uma Vectorclasse com atributos Point ae Point b, onde os únicos métodos são scaleBy(double factor)e printTo(Canvas canvas), ambos operando nos dois atributos. Por outro lado, uma classe minimamente coesa é aquela em que cada atributo é usado em apenas um método e nunca mais de um atributo é usado por cada método. Em média, uma classe presentes não coesivos "grupos" de partes coesas - ou seja, alguns métodos usam atributos a, be c, enquanto o uso de descansoc ed - o que significa que, se dividirmos a classe em duas, acabaremos com dois objetos coesos.

Por fim, as classes Clean devem reduzir o acoplamento o máximo possível. Embora existam muitos tipos de acoplamento que vale a pena discutir aqui, parece que o código em questão sofre principalmente com acoplamento temporal , onde, como você apontou, os métodos do objeto só funcionam conforme o esperado quando são chamados na ordem correta. E, como as duas diretrizes acima mencionadas, as soluções para isso geralmente envolvem a divisão da classe em dois ou mais objetos coesos . A estratégia de divisão nesse caso geralmente envolve padrões como Builder ou Factory e, em casos altamente complexos, State-Machines.

As diretrizes TL; DR: O Código Limpo que seu colega seguiu são boas, mas somente quando também seguem os princípios, práticas e padrões restantes mencionados no livro. A versão limpa da "classe" que você está vendo seria dividida em várias classes, cada uma com uma única responsabilidade, métodos coesos e sem acoplamento temporal. Este é o contexto em que pequenos métodos e argumentos pouco ou nenhum fazem sentido.

MichelHenrich
fonte
1
Você e o topo morto escreveram uma boa resposta, mas só posso aceitar uma. Eu gosto que você tenha abordado SRP, coesão e acoplamento. Estes são termos úteis que posso usar na revisão de código. Dividir o objeto em objetos menores com suas próprias responsabilidades é obviamente o caminho a percorrer. Um método (não construtor) que inicializa valores em um monte de propriedades de classe é uma oferta inoperante de que um novo objeto deve ser retornado. Eu deveria ter visto isso.
user2180613
1
SRP é a orientação mais importante; um anel para governar todos eles e tudo isso. O SRP bem feito resulta naturalmente em métodos mais curtos. Exemplo: Eu tenho uma classe frontal com apenas 2 métodos públicos e cerca de 8 métodos não públicos. Nenhum é mais do que ~ 3 linhas; toda a classe tem cerca de 35 LOC. Mas eu escrevi essa aula por último! No momento em que todo o código subjacente foi escrito, essa classe basicamente se escreveu e eu não precisava, de fato, não poderia aumentar os métodos. Em nenhum momento eu disse "Vou escrever esses métodos em 5 linhas, se isso me matar." Toda vez que você aplica o SRP, isso acontece.
Radarbob # 21/16
11

Entendo que o construtor da classe é totalmente responsável pela criação de um objeto válido e que sua principal tarefa é a atribuição das propriedades (privadas) de um objeto.

Geralmente é responsável por colocar o objeto em um estado válido inicial, sim; outras propriedades ou métodos podem mudar o estado para outro estado válido.

No entanto, no código que encontrei, a maioria dos valores de propriedades é realmente definida por outros métodos que não o construtor. Os valores resultantes dos cálculos são atribuídos a propriedades a serem usadas dentro de vários métodos particulares em toda a classe. Aparentemente, o autor usa propriedades de classe como se fossem variáveis ​​globais que deveriam estar acessíveis em toda a classe, em vez de parametrizar esses valores para as funções que precisam delas. Além disso, os métodos da classe devem ser chamados em uma ordem específica, porque a classe não fará muito de outra maneira.

Assim como os problemas de legibilidade e capacidade de manutenção que você alude, parece que há vários estágios de fluxo / transformação de dados na própria classe, o que pode indicar que a classe está desrespeitando o princípio da responsabilidade única.

suspeite que esse código tenha sido inspirado pelo conselho de manter os métodos curtos (<= 5 linhas de código), para evitar grandes listas de parâmetros (<3 parâmetros) e que os construtores não devem funcionar (como executar um cálculo de algum tipo que é essencial para a validade do objeto).

Seguir algumas diretrizes de codificação e ignorar outras geralmente leva a códigos tolos. Se quisermos evitar que um construtor faça trabalho, por exemplo, a maneira mais sensata seria usualmente fazer o trabalho antes da construção e passar o resultado desse trabalho para o construtor. (Um argumento para essa abordagem pode ser que você está evitando atribuir à sua classe duas responsabilidades: o trabalho de sua inicialização e o seu 'trabalho principal', seja qual for.)

Na minha experiência, tornar as classes e os métodos pequenos raramente é algo que tenho que ter em mente como uma consideração separada - ao contrário, decorre um tanto naturalmente da responsabilidade única.

No entanto, prefiro alterar completamente o código, para que a classe se torne uma cópia azul de um objeto real, em vez de uma série de métodos que devem ser chamados (proceduralmente) em uma ordem específica.

Você provavelmente estaria correto em fazê-lo. Não há nada errado em escrever código processual direto; existe um problema em abusar do paradigma OO para escrever código processual ofuscado.

Acredito que exista uma distinção bastante clara sobre quando salvar um valor em uma propriedade de classe e quando colocá-lo em um parâmetro para um método diferente de usar - eu realmente não acredito que eles possam ser alternativas um para o outro. Estou procurando as palavras para esta distinção.

Normalmente , você não deve colocar um valor em um campo como uma maneira de transmiti-lo de um método para outro; um valor em um campo deve ser uma parte significativa do estado de um objeto em um determinado momento. (Posso pensar em algumas exceções válidas, mas não em que esses métodos são públicos ou em que existe uma dependência de ordem que um usuário da classe deve conhecer)

topo Restabelecer Monica
fonte
2
voto positivo porque: 1. Enfatizando o SRP. "... métodos pequenos ... seguem naturalmente" 2. Finalidade do construtor - estado válido. 3. "Seguir algumas diretrizes de codificação enquanto ignora outras." Este é o Walking Dead da codificação.
Radarbob # 21/16
6

É provável que você se oponha ao padrão errado aqui. Pequenas funções e baixa aridade raramente são problemáticas por si só. A questão real aqui é o acoplamento que causa dependência de ordem entre as funções; portanto, procure maneiras de resolver isso sem desperdiçar os benefícios das pequenas funções.

O código fala mais alto que as palavras. As pessoas recebem esses tipos de correções muito melhor se você puder fazer parte da refatoração e mostrar a melhoria, talvez como um exercício de programação em pares. Quando faço isso, geralmente acho que é mais difícil do que eu pensava em acertar o design, equilibrando todos os critérios.

Karl Bielefeldt
fonte