Quando o paradigma “uma coisa” se torna prejudicial?

21

Por uma questão de argumento, aqui está uma função de exemplo que imprime o conteúdo de um determinado arquivo linha por linha.

Versão 1:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  string line;
  while (std::getline(file, line)) {
    cout << line << endl;
  }
}

Eu sei que é recomendado que funções façam uma coisa em um nível de abstração. Para mim, embora o código acima faça praticamente uma coisa e seja bastante atômico.

Alguns livros (como o Código Limpo de Robert C. Martin) parecem sugerir a quebra do código acima em funções separadas.

Versão 2:

void printFile(const string & filePath) {
  fstream file(filePath, ios::in);
  printLines(file);
}

void printLines(fstream & file) {
  string line;
  while (std::getline(file, line)) {
    printLine(line);
  }
}

void printLine(const string & line) {
  cout << line << endl;
}

Entendo o que eles querem alcançar (arquivo aberto / linhas de leitura / linha de impressão), mas não é um pouco exagerado?

A versão original é simples e, em certo sentido, já faz uma coisa - imprime um arquivo.

A segunda versão levará a um grande número de funções realmente pequenas, que podem ser bem menos legíveis que a primeira versão.

Nesse caso, não seria melhor ter o código em um só lugar?

Em que momento o paradigma "uma coisa" se torna prejudicial?

Petr
fonte
13
Esse tipo de prática de codificação é sempre baseada caso a caso. Nunca existe uma única abordagem.
iammilind
1
@ Alex - A resposta aceita literalmente não tem nada a ver com a pergunta. Acho isso muito estranho.
ChaosPandion
2
Observo que sua versão refatorada está de cabeça para baixo, o que contribui para a falta de legibilidade. Lendo-se o arquivo, você esperaria ver printFile, printLinese, finalmente printLine.
Anthony Pegram
1
@ Kev, mais uma vez só posso discordar, principalmente com essa categorização. Não é pediatria, é o ponto! É o OP que diz especificamente que a segunda versão pode não ser tão legível. É o OP que cita especificamente o Clean Code como inspiração para a segunda versão. Meu comentário é essencialmente que o Clean Code não o faria escrever código dessa maneira. A ordem é realmente importante para facilitar a leitura. Você lê o arquivo como lê um artigo de jornal, obtendo cada vez mais detalhes até basicamente ficar desinteressado.
Anthony Pegram
1
Como você não esperaria ler um poema de trás para frente, nem esperaria ver o nível mais baixo de detalhes como a primeira coisa em uma classe específica. Na sua opinião, esse código leva pouco tempo para ser resolvido rapidamente, mas eu presumo que esse código não seja o único que ele escreverá. Para o meu ponto, se ele citar o Código Limpo, o mínimo que ele poderia fazer é segui-lo. Se o código estiver com defeito, certamente será menos legível do que o contrário.
Anthony Pegram

Respostas:

15

Obviamente, isso apenas sugere a pergunta "O que é uma coisa?" Ler uma linha é uma coisa e escrever uma linha é outra? Ou copiar uma linha de um fluxo para outro é considerado uma coisa? Ou copiando um arquivo?

Não existe uma resposta objetiva e difícil. Você decide. Você pode decidir. Você tem que decidir. O principal objetivo do paradigma "faça uma coisa" é provavelmente produzir código o mais fácil de entender possível, para que você possa usá-lo como uma diretriz. Infelizmente, isso também não é mensurável objetivamente, então você deve confiar no seu instinto e no "WTF?" conte na revisão de código .

Uma função IMO que consiste em apenas uma única linha de código raramente vale a pena. Você printLine()não tem vantagem em usar std::cout << line << '\n'1 diretamente. Se eu printLine()vir, devo assumir que ele faz o que o nome diz ou procurar e verificar. Se eu vir std::cout << line << '\n', eu sei imediatamente o que faz, porque esta é a maneira canônica de exibir o conteúdo de uma string como uma linha para std::cout.

No entanto, outro objetivo importante do paradigma é permitir a reutilização de código, e essa é uma medida muito mais objetiva. Por exemplo, na sua segunda versão, printLines() poderia ser facilmente escrito, de modo que é um algoritmo universalmente útil que copia linhas de um fluxo para outro:

void copyLines(std::istream& is, std::ostream& os)
{
  std::string line;
  while( std::getline(is, line) );
    os << line << '\n';
  }
}

Esse algoritmo também poderia ser reutilizado em outros contextos.

Você pode colocar tudo o que é específico desse caso de uso em uma função que chama esse algoritmo genérico:

void printFile(const std::string& filePath) {
  std::ifstream file(filePath.c_str());
  printLines(file, std::cout);
}

1 Observe que eu usei em '\n'vez de std::endl. '\n'deve ser a opção padrão para a saída de uma nova linha , std::endlé o caso ímpar .

sbi
fonte
2
+1 - concordo principalmente, mas acho que há mais do que "pressentimento". O problema é quando as pessoas julgam "uma coisa" contando detalhes da implementação. Para mim, a função deve implementar (e seu nome descrever) uma única abstração clara. Você nunca deve nomear uma função "do_x_and_y". A implementação pode e deve fazer várias coisas (mais simples) - e cada uma dessas coisas mais simples pode ser decomposta em várias coisas ainda mais simples e assim por diante. É apenas uma decomposição funcional com uma regra extra - que as funções (e seus nomes) descrevam um único conceito / tarefa / qualquer coisa clara.
Steve314
@ Steve314: Não listei os detalhes da implementação como possibilidades. Copiar linhas de um fluxo para outro claramente é uma abstração de uma coisa . Ou é? E é fácil evitar do_x_and_y()nomear a função do_everything(). Sim, esse é um exemplo bobo, mas mostra que essa regra nem sequer impede os exemplos mais extremos de mau design. Na OMI, essa é uma decisão que parece tanto quanto ditada por convenções. Caso contrário, se fosse objetivo, você poderia criar uma métrica para ela - o que não pode.
SBI
1
Eu não pretendia contradizer - apenas sugerir uma adição. Eu acho que o que eu esqueci de dizer é que, a partir da pergunta, a decomposição para printLineetc é válida - cada uma delas é uma única abstração - mas isso não significa necessário. printFilejá é "uma coisa". Embora você possa decompor isso em três abstrações de nível inferior separadas, não é necessário decompor em todos os níveis possíveis de abstração. Cada função deve fazer "uma coisa", mas nem todas as possíveis "uma coisa" precisam ser uma função. Mover muita complexidade para o gráfico de chamadas pode ser um problema.
Steve314
7

Ter uma função fazendo apenas "uma coisa" é um meio para dois fins desejáveis, não um mandamento de Deus:

  1. Se sua função fizer apenas "uma coisa", isso ajudará a evitar a duplicação de código e o inchaço da API, pois você poderá compor funções para realizar tarefas mais complexas, em vez de ter uma explosão combinatória de funções de nível superior e menos composíveis .

  2. Ter funções fazendo apenas "uma coisa" pode tornar o código mais legível. Isso depende se você obtém mais clareza e facilidade de raciocínio ao dissociar as coisas do que perde com a verbosidade, a indireção e a sobrecarga conceitual das construções que permitem dissociar as coisas.

Portanto, "uma coisa" é inevitavelmente subjetiva e depende de qual nível de abstração é relevante para o seu programa. Se printLinesfor considerada uma operação única e fundamental e a única maneira de imprimir linhas com as quais você se preocupa ou prevê se preocupar, então, para seus propósitos, printLinesapenas uma coisa é necessária. A menos que você ache a segunda versão mais legível (não acho), a primeira versão está correta.

Se você começar a precisar de mais controle sobre os níveis mais baixos de abstração e terminar com duplicação sutil e explosão combinatória (ou seja, um printLinespara nomes de arquivos e um completamente separado printLinespara fstreamobjetos, um printLinespara console e um printLinespara arquivos), printLinesestará fazendo mais de uma coisa no nível de abstração que você gosta.

dsimcha
fonte
Eu acrescentaria uma terceira e que funções menores são mais facilmente testadas. Como provavelmente há menos entradas necessárias se a função fizer apenas uma coisa, torna mais fácil testá-la independentemente.
PersonalNexus
@ PersonalalNexus: Concordo um pouco com a questão dos testes, mas o IMHO é tolice testar os detalhes da implementação. Para mim, um teste de unidade deve testar "uma coisa", conforme definido na minha resposta. Qualquer coisa mais refinada está tornando seus testes frágeis (porque a alteração dos detalhes da implementação exigirá que seus testes sejam alterados) e seu código irritantemente detalhado, indireto etc. (porque você adicionará a indireção apenas para dar suporte aos testes).
precisa
6

Nesta escala, não importa. A implementação de função única é perfeitamente óbvia e compreensível. No entanto, adicionar um pouco mais de complexidade torna muito atraente dividir a iteração da ação. Por exemplo, suponha que você precise imprimir linhas de um conjunto de arquivos especificado por um padrão como "* .txt". Então eu separaria a iteração da ação:

printLines(FileSet files) {
   files.each({ 
       file -> file.eachLine({ 
           line -> printLine(line); 
       })
   })
}

Agora a iteração do arquivo pode ser testada separadamente.

Dividi funções para simplificar o teste ou melhorar a legibilidade. Se a ação executada em cada linha de dados fosse complexa o suficiente para justificar um comentário, eu certamente a dividiria em uma função separada.

Kevin Cline
fonte
4
Eu acho que você acertou em cheio. Se precisamos de um comentário para explicar uma linha, sempre é hora de extrair um método.
Roger CS Wernersson
5

Extraia métodos quando achar necessário comentar para explicar as coisas.

Escreva métodos que façam apenas o que o nome diz de maneira óbvia ou conte uma história chamando métodos com nomes inteligentes.

Roger CS Wernersson
fonte
3

Mesmo no seu caso simples, faltam detalhes que o Princípio de Responsabilidade Única o ajudaria a gerenciar melhor. Por exemplo, o que acontece quando algo dá errado ao abrir o arquivo. Adicionar um tratamento de exceção para proteção contra casos de borda de acesso a arquivos adicionaria 7 a 10 linhas de código à sua função.

Depois de abrir o arquivo, você ainda não está seguro. Pode ser arrancado de você (especialmente se for um arquivo em uma rede), você pode ficar sem memória. Novamente, vários casos extremos podem ocorrer contra os quais você deseja proteger e inchar sua função monolítica.

A linha de impressão de uma linha parece inócua o suficiente. Porém, à medida que novas funcionalidades são adicionadas à impressora de arquivos (análise e formatação de texto, renderização para diferentes tipos de exibição etc.), ela cresce e você agradecerá mais tarde.

O objetivo do SRP é permitir que você pense em uma única tarefa de cada vez. É como dividir um grande bloco de texto em vários parágrafos para que o leitor possa compreender o ponto que você está tentando entender. Leva um pouco mais de tempo para escrever um código que cumpra esses princípios. Mas, ao fazer isso, facilitamos a leitura desse código. Pense em como o seu futuro será feliz quando ele precisar rastrear um bug no código e o encontrar bem particionado.

Michael Brown
fonte
2
Votei positivamente nesta resposta porque gosto da lógica, embora discorde dela! Fornecer estrutura com base em um pensamento complexo sobre o que pode acontecer no futuro é contraproducente. Fatorar código quando você precisar. Não abstraia as coisas até precisar. O código moderno é atormentado por pessoas que tentam seguir regras servilmente, em vez de apenas escrever um código que funcione e adaptá-lo com relutância . Bons programadores são preguiçosos .
Yttrill
Obrigado pelo comentário. Observe que não estou defendendo abstraculação prematura, apenas dividindo operações lógicas para que seja mais fácil fazê-lo mais tarde.
Michael Brown
2

Pessoalmente, prefiro a última abordagem, porque economiza o trabalho no futuro e força a mentalidade de "como fazê-lo de maneira genérica". Apesar disso, no seu caso, a Versão 1 é melhor que a Versão 2 - apenas porque os problemas resolvidos pela Versão 2 são muito triviais e específicos do fluxo. Eu acho que deve ser feito da seguinte maneira (incluindo correção de bug proposta por Nawaz):

Funções utilitárias genéricas:

void printLine(ostream& output, const string & line) { 
    output << line << endl; 
} 

void printLines(istream& input, ostream& output) { 
    string line; 
    while (getline(input, line)) {
        printLine(output, line); 
    } 
} 

Função específica do domínio:

void printFile(const string & filePath, ostream& output = std::cout) { 
    fstream file(filePath, ios::in); 
    printLines(file, output); 
} 

Agora printLinese printLinepode trabalhar não apenas com fstream, mas com qualquer fluxo.

gwiazdorrr
fonte
2
Discordo. Essa printLine()função não tem valor. Veja minha resposta .
Sbi
1
Bem, se mantivermos printLine (), podemos adicionar um decorador que adicione números de linhas ou cores de sintaxe. Dito isto, eu não extrairia esses métodos até encontrar um motivo.
Roger CS Wernersson
2

Todo paradigma (não necessariamente o que você citou) a ser seguido exige alguma disciplina e, assim, reduz a "liberdade de expressão" - resulta em uma sobrecarga inicial (pelo menos apenas porque você precisa aprender!). Nesse sentido, todo paradigma pode se tornar prejudicial quando o custo dessa sobrecarga não é supercompensado pela vantagem que o paradigma é projetado para manter consigo mesmo.

A verdadeira resposta à pergunta, portanto, requer uma boa capacidade de "prever" o futuro, como:

  • Estou agora obrigado a fazer AeB
  • Qual é a probabilidade, em um futuro próximo eu também precisarei fazer A-e B+(isto é, algo que se parece com A e B, mas um pouco diferente)?
  • Qual é a probabilidade em um futuro mais distante, de que o A + se torne A*ou A*-?

Se essa probabilidade for relativamente alta, será uma boa chance se, ao pensar em A e B, também pensar em suas possíveis variantes, para isolar as partes comuns para poder reutilizá-las.

Se essa probabilidade for muito baixa (qualquer que seja a variante ao redor Anada mais é do que Aela mesma), estude como decompor A mais provavelmente resultará em perda de tempo.

Apenas como exemplo, deixe-me contar uma história real:

Durante minha vida passada como um professor, eu descobri que -na maioria dos projetos- do aluno praticamente todos eles fornecem a sua própria função para calcular o comprimento de uma string C .

Após alguma investigação, descobri que, sendo um problema frequente, todos os alunos tiveram a ideia de usar uma função para isso. Depois de dizer a eles que existe uma função de biblioteca para isso ( strlen), muitos deles responderam que, como o problema era tão simples e trivial, era mais eficaz escrever sua própria função (2 linhas de código) do que procurar o manual da biblioteca C (em 1984, esqueci a WEB e o google!) em ordem alfabética estrita para ver se havia uma função pronta para isso.

Este é um exemplo em que também o paradigma "não reinvente a roda" pode se tornar prejudicial, sem um catálogo de rodas eficaz!

Emilio Garavaglia
fonte
2

Seu exemplo é bom para ser usado em uma ferramenta descartável necessária ontem para executar alguma tarefa específica. Ou como uma ferramenta de administração controlada diretamente por um administrador. Agora, torne-o robusto para ser adequado para seus clientes.

Adicione manipulação adequada de erro / exceção com mensagens significativas. Talvez você precise da verificação de parâmetros, incluindo decisões que devem ser tomadas, por exemplo, como lidar com arquivos não existentes. Adicione a funcionalidade de log, talvez com níveis diferentes, como informações e depuração. Adicione comentários para que seus colegas de equipe saibam o que está acontecendo lá. Adicione todas as partes geralmente omitidas por brevidade e deixadas como exercício para o leitor ao fornecer exemplos de código. Não esqueça seus testes de unidade.

Sua pequena e agradável função linear de repente termina em uma confusão complexa que implora para ser dividida em funções separadas.

Seguro
fonte
2

Na OMI, torna-se prejudicial quando chega tão longe que uma função quase nada faz, exceto delegar trabalho a outra função, porque isso é um sinal de que não é mais uma abstração de nada e que a mentalidade que leva a essas funções está sempre em risco de ocorrer. fazendo coisas piores ...

Da postagem original

void printLine(const string & line) {
  cout << line << endl;
}

Se você for pedante o suficiente, poderá notar que o printLine ainda faz duas coisas: escrever a linha para cout e adicionar um caractere "linha final". Algumas pessoas podem querer lidar com isso criando novas funções:

void printLine(const string & line) {
  reallyPrintLine(line);
  addEndLine();
}

void reallyPrintLine(const string & line) {
  cout << line;
}

void addEndLine() {
  cout << endl;
}

Oh não, agora nós tornamos o problema ainda pior! Agora é óbvio que o printLine faz DUAS coisas !!! 1! Não é muita estupidez criar as "soluções alternativas" mais absurdas que se pode imaginar para se livrar do problema inevitável de que imprimir uma linha consiste em imprimir a própria linha e adicionar um caractere de fim de linha.

void printLine(const string & line) {
  for (int i=0; i<2; i++)
    reallyPrintLine(line, i);
}

void reallyPrintLine(const string & line, int action) {
  cout << (action==0?line:endl);
}
user281377
fonte
1

Resposta curta ... depende.

Pense nisso: e se, no futuro, você não desejar imprimir apenas na saída padrão, mas em um arquivo.

Eu sei o que é o YAGNI, mas só estou dizendo que pode haver casos em que algumas implementações são conhecidas por serem necessárias, mas adiadas. Portanto, talvez o arquiteto ou o que quer que saiba que essa função precise ser capaz de imprimir também em um arquivo, mas não queira fazer a implementação no momento. Então ele cria essa função extra para que, no futuro, você só precise alterar a saída em um único local. Faz sentido?

No entanto, se você tiver certeza de que precisa apenas de saída no console, isso não faz muito sentido. Escrever um "invólucro" cout <<parece inútil.

Luchian Grigore
fonte
1
Mas, estritamente falando, a função printLine não é um nível diferente de abstração da iteração sobre linhas?
@ Pet Acho que sim, e é por isso que sugerem que você separe a funcionalidade. Eu acho que o conceito está correto, mas você precisa aplicá-lo caso a caso.
1

A razão pela qual existem livros dedicando capítulos às virtudes de "fazer uma coisa" é que ainda existem desenvolvedores por aí que escrevem funções com 4 páginas e aninhando 6 níveis condicionais. Se o seu código é simples e claro, você fez o que é certo.

Kevin
fonte
0

Como outros pôsteres comentaram, fazer uma coisa é uma questão de escala.

Eu também sugeriria que a idéia de uma coisa é parar as pessoas de codificar por efeito colateral. Isso é exemplificado pelo acoplamento seqüencial, em que os métodos precisam ser chamados em uma ordem específica para obter o resultado "certo".

NWS
fonte