Não há problema em dividir funções e métodos longos em funções menores, mesmo que eles não sejam chamados por mais nada? [duplicado]

O código de teste que faz muitas coisas é difícil.

O código de depuração que faz muitas coisas é difícil.

A solução para esses dois problemas é escrever código que não faz muitas coisas. Escreva cada função para que ela faça uma coisa e apenas uma coisa. Isso os torna fáceis de testar com um teste de unidade (não é necessário um número de dúzias de testes unitários).

Um colega meu tem a frase que ele usa ao julgar se um determinado método precisa ser dividido em outros menores:

If, when describing the activity of the code to another programmer you use the word 'and', the method needs to be split into at least one more part.

Você escreveu:

I have a process_url() function which splits URLs into components and then assigns them to some objects via their methods.

Isso deve ser pelo menos dois métodos. Não há problema em envolvê-los em um método de exibição pública, mas o funcionamento deve ser de dois métodos diferentes.

Sim, a divisão de funções longas é normal. Esta é uma maneira de fazer as coisas que são encorajadas por Robert C. Martin em seu livro Clean Code . Particularmente, você deve escolher nomes muito descritivos para suas funções, como uma forma de código de auto-documentação.

Como as pessoas apontaram, isso melhora a legibilidade. Uma pessoa lendo process_url() pode ver com mais clareza qual é o processo geral para lidar com URLs apenas lendo alguns nomes de métodos.

O problema é que outras pessoas podem pensar que essas funções são usadas por alguma outra parte do código, e se algumas delas precisam ser alteradas, elas podem optar por preservar essas funções e definir novas. Isso significa que algum código fica inacessível.

Existem várias maneiras de lidar com isso. A primeira é a documentação e comentários no código. Em segundo lugar, ferramentas que fornecem testes de cobertura. Em qualquer caso, em grande parte isso depende da linguagem de programação, estas são algumas das soluções que você pode aplicar, dependendo da linguagem de programação:

• linguagens orientadas a objetos podem permitir definir alguns métodos privados, para garantir que eles não sejam usados em outros lugares
• módulos em outros idiomas podem especificar quais funções são visíveis de fora, etc.
• linguagens de alto nível, como o Python, podem eliminar a necessidade de definir várias funções, porque, de qualquer maneira, elas seriam simples para forros.
• outras linguagens como o Prolog podem exigir (ou sugerir strongmente) a definição de um novo predicado para cada salto condicional.
• em alguns casos é comum definir funções auxiliares dentro da função que as utiliza (funções locais), às vezes são funções anônimas (fechamento de código), isso é comum em funções de callback Javascript.

Portanto, em resumo, a divisão em várias funções geralmente é uma boa ideia em termos de legibilidade. Pode não ser realmente bom se as funções forem muito curtas e isso criar o efeito goto ou se os nomes não forem realmente descritivos; nesse caso, a leitura de algum código exigiria saltos entre funções, o que pode ser confuso. Sobre a sua preocupação com o escopo e o uso dessas funções, existem várias maneiras de lidar com isso que dependem do idioma geral.

Em geral, o melhor conselho é usar o bom senso. Qualquer regra estrita é muito provável que esteja errada em alguns casos e no final depende da pessoa. Eu consideraria isso legível:

process_url = lambda url: dict(re.findall('([^?=&]*)=([^?=&]*)', url))

Portanto, a palavra-chave é equilíbrio . Uma função com mil linhas é um inferno para quem lê, porque não há encapsulamento e o contexto se torna muito grande. Uma função dividida em mil funções, cada uma delas com uma linha, pode ser pior:

• você tem alguns nomes (que você poderia ter fornecido como comentários nas linhas)
• você está (esperançosamente) eliminando variáveis globais e não precisa se preocupar com o estado (ter transparência referencial)
• mas você força os leitores a pular para frente e para trás.

Portanto, não há balas de prata aqui, mas experiência e equilíbrio. IMHO a melhor maneira de aprender como fazer isso é ler um monte de código escrito por outras pessoas analisando por que é difícil para você lê-lo e como torná-lo mais legível. Isso proporcionaria uma valiosa experiência.

Eu diria que isso depende.

Se você está apenas dividindo-o para dividir e chamar nomes como process_url_partN e assim por diante, então NÃO , por favor, não. Isso só torna mais difícil seguir mais tarde quando você ou outra pessoa precisa descobrir o que está acontecendo.

Se você estiver usando métodos com finalidades claras que possam ser testados por eles mesmos e façam sentido sozinhos (mesmo que ninguém mais os esteja usando), então YES .

Para o seu propósito particular, parece que você tem dois objetivos.

1. Analise um URL e retorne uma lista de seus componentes.
2. Faça algo com esses componentes.

Eu escreveria a primeira parte separada e faria com que ela retornasse um resultado geral que poderia ser facilmente testado e potencialmente reutilizado posteriormente. Melhor ainda, eu procuraria por uma função interna que já faz isso em sua linguagem / framework e use isso, a menos que sua análise seja super especial. Se for super especial, ainda o escreveria como um método separado, mas provavelmente "empacotá-lo" como um método privado / protegido na classe que lida com o segundo (se você estiver escrevendo código orientado a objeto).

A segunda parte que eu escreveria como seu próprio componente, que usa o primeiro para a análise de URL.

Eu nunca tive problema com outros desenvolvedores dividindo métodos maiores em métodos menores, já que é um padrão que eu mesmo me acompanho. O método "Deus" é uma armadilha terrível para cair e outros que são menos experientes ou simplesmente não se importam tendem a ser pegos com mais freqüência do que não. Dito isto ...

É incrivelmente importante usar identificadores de acesso apropriados nos métodos menores. É frustrante encontrar uma classe repleta de pequenos métodos públicos, porque então eu perco totalmente a confiança para encontrar onde / como o método é usado em todo o aplicativo.

Eu moro em C # -land, então temos public , private , protected , internal , e ver essas palavras me mostra, sem sombra de dúvida, o escopo do método e onde devo procurar para chamadas. Se for particular, sei que o método é usado em apenas uma classe e tenho total confiança na refatoração.

No mundo do Visual Studio, ter várias soluções ( .sln ) exacerba esse antipadrão, pois os auxiliares "Localizar usos" do IDE / resharper não encontrarão usos fora da solução aberta.

Se a sua linguagem de programação oferecer suporte, você poderá definir suas funções "auxiliares" dentro do escopo de sua função process_url() para obter os benefícios de legibilidade de funções separadas. por exemplo,

function process_url(url) {

    function foo(a) {
        // ...
    }

    function bar(a) {
        // ...
    }

    return [ foo(url), bar(url) ];

}

Se a sua linguagem de programação não der suporte a isso, você poderá mover foo() e bar() para fora do escopo de process_url() (para que fique visível para outras funções / métodos) - mas considere isso como " hackear "você colocou em prática porque sua linguagem de programação não suporta este recurso.

Se a quebra de uma função em subfunções provavelmente dependerá da existência de nomes significativos / úteis para as partes e da extensão de cada uma das funções, entre outras considerações.

Se alguém estiver interessado em alguma literatura sobre esta questão: Isto é exatamente o que Joshua Kerievsky se refere como "Compose Method" em seu "Refactoring to Patterns" (Addison-Wesley):

Transform the logic into a small number of intention-revealing steps at the same level of detail.

Eu acredito que o aninhamento correto de métodos de acordo com seu "nível de detalhes" é importante aqui.

Veja um extrato no site do editor:

Much of the code we write doesn’t start out being simple. To make it simple, we must reflect on what isn’t simple about it and continually ask, “How could it be simpler?” We can often simplify code by considering a completely different solution. The refactorings in this chapter present different solutions for simplifying methods, state transitions, and tree structures.

Compose Method (123) is about producing methods that efficiently communicate what they do and how they do what they do. A Composed Method [Beck, SBPP] consists of calls to well-named methods that are all at the same level of detail. If you want to keep your system simple, endeavor to apply Compose Method (123) everywhere...

Adendo: Kent Beck ( "Padrões de Implementação" ) refere-se a como "Método Composto". Ele aconselha você a:

[c]ompose methods out of calls to other methods, each of which is at roughly the same level of abstraction. One of the signs of a poorly composed method is a mixture of abstraction levels[.]

Lá, novamente, o aviso para não misturar diferentes níveis de abstração (ênfase minha).

Uma boa regra é ter abstrações próximas em níveis semelhantes (melhor formuladas por sebastian em responda logo acima.)

Ou seja. se você tem uma função (grande) que lida com coisas de baixo nível, mas faça algumas escolhas de nível mais alto também, tente fatorar as coisas de baixo nível:

void foo() {

     if(x) {
       y = doXformanysmallthings();
     }

     z = doYforthings(y);

     if (z != y && isFullmoon()) {
         launchSpacerocket()
     }
}

Mover coisas para funções menores geralmente é melhor do que ter um monte de loops e dentro de uma função que consista em alguns passos "grandes" conceituais. (A menos que você possa combinar isso em expressões LINQ / foreach / lambda relativamente pequenas ...)

Se você pudesse criar uma classe apropriada para essas funções, torná-las privadas. Em outras palavras, com uma definição de classe adequada, você pode expor apenas o que precisa expor.

Tenho certeza de que essa não será a opinião popular, mas está tudo bem. A Localidade de Referência pode ser uma grande ajuda para garantir que você e os outros entendam a função (neste caso, estou me referindo ao código e não especificamente à memória).

Como em tudo, é um equilíbrio. Você deve estar mais preocupado com alguém que diga "sempre" ou "nunca".

Considere esta função simples (estou usando a sintaxe semelhante ao Scala, mas espero que a ideia seja clara sem qualquer conhecimento do Scala):

def myFun ... {
    ...
    if (condition1) {
        ...
    } else {
        ...
    }
    if (condition2) {
        ...
    } else {
        ...
    }
    if (condition3) {
        ...
    } else {
        ...
    }
    // rest
    ...
}

Esta função tem até 8 caminhos possíveis, como o seu código pode ser executado, dependendo de como essas condições são avaliadas.

• Isso significa que você precisará de 8 testes diferentes apenas para testar essa parte. Além disso, muito provavelmente algumas combinações não serão possíveis, e então você terá que analisar cuidadosamente quais são elas (e tenha certeza de não perder algumas que são possíveis) - muito trabalho.
• É muito difícil raciocinar sobre o código e sua correção. Como cada um dos blocos if e sua condição podem depender de algumas variáveis locais compartilhadas, para saber o que está acontecendo depois deles, todo mundo que trabalha com o código deve analisar todos os blocos de código e os 8 caminhos de execução possíveis. É muito fácil cometer um erro aqui e, provavelmente, alguém que está atualizando o código perderá algo e apresentará um bug.

Por outro lado, se você estruturar o cálculo como

def myFun ... {
    ...
    val result1 = myHelper1(...);
    val result2 = myHelper2(...);
    val result3 = myHelper3(...);
    // rest
    ...
}

private def myHelper1(/* arguments */): SomeResult = {
    if (condition1) {
        ...
    } else {
        ...
    }
}
// Similarly myHelper2 and myHelper3

você pode:

• Teste facilmente cada uma das funções auxiliares, cada uma delas tem apenas dois caminhos possíveis de execução.
• Ao examinar myFun , fica imediatamente óbvio se result2 depende de result1 (apenas verificando se a chamada para myHelper2(...) o utiliza para computar um dos argumentos. (Supondo que os ajudantes não usem alguns Também é óbvio como eles são dependentes, algo que é muito mais difícil de entender no caso anterior. Além disso, após as três chamadas, também fica claro como o estado da computação parece - é capturado apenas em result1 , result2 e result3 - não há necessidade de verificar se / quais outras variáveis locais foram modificadas.

A responsabilidade mais concreta de um método, mais fácil de testar, ler e manter, será o código. Embora nenhum outro ligue para eles.

Se, no futuro, você precisar usar essa funcionalidade de outros lugares, será fácil extrair esses métodos.

É perfeitamente aceitável nomear seus métodos assim:

MyProcedure()
  MyProcedure_part1()
  MyProcedure_part2()
end;

Eu não vejo por que alguém pensaria que isso seria chamado por algum outro processo, e está perfeitamente claro para que servem.