Eu já vi isso muito em nosso sistema legado em funcionamento - funções que são mais ou menos assim:
bool todo = false;
if(cond1)
{
... // lots of code here
if(cond2)
todo = true;
... // some other code here
}
if(todo)
{
...
}
Em outras palavras, a função possui duas partes. A primeira parte realiza algum tipo de processamento (potencialmente contendo loops, efeitos colaterais etc.) e, ao longo do caminho, pode definir o sinalizador "todo". A segunda parte é executada apenas se o sinalizador "todo" tiver sido definido.
Parece uma maneira muito feia de fazer as coisas, e acho que a maioria dos casos que realmente demorei a entender, poderia ser refatorada para evitar o uso da bandeira. Mas isso é um anti-padrão real, uma má idéia ou perfeitamente aceitável?
A primeira refatoração óbvia seria dividi-la em dois métodos. No entanto, minha pergunta é mais sobre se existe a necessidade (em uma linguagem OO moderna) de criar uma variável de flag local, potencialmente configurando-a em vários locais e, em seguida, usá-la posteriormente para decidir se deve executar o próximo bloco de código.
["blacklisted-domain","suspicious-characters","too-long"]
essa que mostra que várias razões foram aplicadas.Respostas:
Eu não sei sobre anti-padrão, mas extrairia três métodos disso.
O primeiro executaria algum trabalho e retornaria um valor booleano.
O segundo executaria qualquer trabalho executado por "algum outro código"
O terceiro executaria o trabalho auxiliar se o booleano retornado fosse verdadeiro.
Os métodos extraídos provavelmente seriam privados se fosse importante que o segundo apenas (e sempre) fosse chamado se o primeiro método retornasse verdadeiro.
Nomeando bem os métodos, espero que isso torne o código mais claro.
Algo assim:
Obviamente, há um debate sobre se os retornos iniciais são aceitáveis, mas esse é um detalhe da implementação (como é o padrão de formatação de código).
O ponto é que a intenção do código se torna mais clara, o que é bom ...
Um dos comentários sobre a questão sugere que esse padrão representa um cheiro , e eu concordo com isso. Vale a pena olhar para ver se você pode tornar a intenção mais clara.
fonte
todo
variável e provavelmente seria mais difícil de entender.if (check_if_needed ()) do_whatever ();
, não há bandeira óbvia lá. Eu acho que isso pode quebrar muito o código e potencialmente prejudicar a legibilidade se o código for razoavelmente simples. Afinal, os detalhes do que você fazdo_whatever
podem afetar a forma como você testacheck_if_needed
, de modo que é útil manter todo o código junto na mesma tela. Além disso, isso não garante quecheck_if_needed
possa evitar o uso de um sinalizador - e, se o fizer, provavelmente usará váriasreturn
instruções para fazê-lo, possivelmente perturbando os advogados estritos de saída única.... // some other code here
no caso regresso antecipadoEu acho que a feiúra se deve ao fato de haver muito código em um único método, e / ou variáveis serem mal nomeadas (ambas com cheiro de código por direito próprio - antipadrões são coisas mais abstratas e complexas da IMO).
Portanto, se você extrair a maior parte do código em métodos de nível inferior, como o @Bill sugere, o restante ficará limpo (pelo menos para mim). Por exemplo
Ou você pode até se livrar completamente da bandeira local, ocultando a verificação de bandeira no segundo método e usando um formulário como
No geral, não vejo uma variável de flag local como necessariamente ruim por si só. Qual opção acima é mais legível (= preferível para mim) depende do número de parâmetros do método, dos nomes escolhidos e de qual forma é mais consistente com a lógica interna do código.
fonte
todo é um nome muito ruim para a variável, mas acho que pode estar tudo errado. É difícil ter certeza absoluta sem o contexto.
Digamos que a segunda parte da função classifique uma lista, criada pela primeira parte. Isso deve ser muito mais legível:
No entanto, a sugestão de Bill também está correta. Ainda é mais legível:
fonte
O padrão da máquina de estado parece bom para mim. Os anti padrões lá são "todo" (nome incorreto) e "muito código".
fonte
Depende mesmo. Se o código guardado por
todo
(espero que você não esteja usando esse nome de verdade, pois é totalmente não-mnemônico!) É um código de limpeza conceitual, então você tem um antipadrão e deve usar algo como RAII ou C # de C ++using
construir para lidar com as coisas em seu lugar.Por outro lado, se conceitualmente não for um estágio de limpeza, mas apenas um processamento adicional que às vezes é necessário e onde a decisão de fazê-lo precisa ser tomada mais cedo, o que está escrito está bem. Considere se os blocos de código individuais seriam melhor refatorados para suas próprias funções, é claro, e também se você capturou o significado da variável flag em seu nome, mas esse padrão básico de código está OK. Em particular, tentar colocar demais em outras funções pode deixar o que está acontecendo menos claro, e isso definitivamente seria um antipadrão.
fonte
Muitas das respostas aqui teriam problemas para passar em uma verificação de complexidade, algumas pareciam> 10.
Eu acho que essa é a parte "antipadrão" do que você está vendo. Encontre uma ferramenta que mede a complexidade ciclomática do seu código - existem plugins para o eclipse. É essencialmente uma medida de quão difícil seu código é testar e envolve o número e os níveis de ramificações de código.
Como um palpite total sobre uma possível solução, o layout do seu código me faz pensar em "Tarefas", se isso acontecer em muitos lugares, talvez o que você realmente queira seja uma arquitetura orientada a tarefas - cada tarefa sendo sua objeto e no meio da tarefa, você tem a capacidade de enfileirar a próxima tarefa instanciando outro objeto de tarefa e jogando-o na fila. Eles podem ser incrivelmente simples de configurar e reduzem significativamente a complexidade de certos tipos de código - mas como eu disse, essa é uma facada total no escuro.
fonte
Usando o exemplo de pdr acima, como é um bom exemplo, darei um passo adiante.
Ele tinha:
Então percebi que o seguinte funcionaria:
Mas não é tão claro.
Então, para a pergunta original, por que não ter:
E deixe SortList decidir se requer classificação?
Você vê que o método BuildList parece saber sobre classificação, pois retorna um bool indicando como tal, mas isso não faz sentido para um método projetado para criar uma lista.
fonte
Sim, isso parece ser um problema, porque você precisa continuar acompanhando todos os lugares em que está marcando a bandeira como LIGADA / DESLIGADA. É melhor incluir a lógica apenas dentro como uma condição aninhada, em vez de retirar a lógica.
Também modelos de domínio sofisticados, nesse caso, apenas um liner fará grandes coisas dentro do objeto.
fonte
Se o sinalizador for definido apenas uma vez, mova o código
...
para imediatamente após o
... // algum outro código aqui
e elimine o sinalizador.
Caso contrário, divida o
... // muitos códigos aqui
... // algum outro código aqui
...
codifique em funções separadas, se possível, portanto é claro que essa função tem uma responsabilidade que é a lógica da ramificação.
Sempre que possível, separe o código dentro de
... // muitos códigos aqui
descritos em duas ou mais funções, algumas que funcionam (que é um comando) e outras que retornam o valor todo (que é uma consulta) ou o fazem muito explícito, eles a estão modificando (uma consulta usando efeitos colaterais)
O código em si não é o anti-padrão que está acontecendo aqui ... Suspeito que a lógica de ramificação combinada com a execução real de coisas (comandos) seja o anti-padrão que você está procurando.
fonte
Às vezes, acho que preciso implementar esse padrão. Às vezes, você deseja executar várias verificações antes de prosseguir com uma operação. Por razões de eficiência, os cálculos que envolvem determinadas verificações não são feitos, a menos que pareça absolutamente necessário. Então você normalmente vê códigos como:
Isso pode ser simplificado separando a validação do processo real de execução da operação, para que você veja mais como:
Obviamente, varia de acordo com o que você está tentando alcançar, embora seja escrito assim, tanto o código de "sucesso" quanto o código de "falha" são escritos uma vez, o que simplifica a lógica e mantém o mesmo nível de desempenho. A partir daí, seria uma boa ideia ajustar níveis inteiros de validação em métodos internos que retornam indicações booleanas de sucesso ou falha que simplificam ainda mais as coisas, embora alguns programadores gostem de métodos extremamente longos por algum motivo estranho.
fonte
Este não é um padrão . A interpretação mais geral é que você está definindo uma variável booleana e ramificando seu valor posteriormente. Isso é programação processual normal, nada mais.
Agora, seu exemplo específico pode ser reescrito como:
Isso pode ser mais fácil de ler. Ou talvez não. Depende do restante do código que você omitiu. Concentre-se em tornar esse código mais sucinto.
fonte
Os sinalizadores locais usados para o fluxo de controle devem ser reconhecidos como uma forma de
goto
disfarce. Se um sinalizador for usado apenas dentro de uma função, ele poderá ser eliminado gravando duas cópias da função, rotulando um como "sinalizador é verdadeiro" e o outro como "sinalizador é falso" e substituindo todas as operações que configuram o sinalizador quando está claro, ou apaga quando está definido, com um salto entre as duas versões da função.Em muitos casos, o código que usa o uso de um sinalizador será mais limpo do que qualquer alternativa possível que use
goto
, mas isso nem sempre é verdade. Em alguns casos, usargoto
para pular um pedaço de código pode ser mais limpo do que usar sinalizadores para fazê-lo [embora algumas pessoas possam inserir um certo desenho animado do raptor aqui].Penso que o principal princípio orientador deve ser que o fluxo lógico do programa se assemelhe à descrição da lógica de negócios na medida do possível. Se os requisitos da lógica de negócios forem descritos em termos de estados que se dividem e se fundem de maneiras estranhas, ter a lógica do programa da mesma forma pode ser mais limpo do que tentar usar sinalizadores para ocultar essa lógica. Por outro lado, se a maneira mais natural de descrever regras de negócios for dizer que uma ação deve ser executada se outras ações tiverem sido executadas, a maneira mais natural de expressar isso pode ser usar um sinalizador que é definido ao executar as últimas ações e é claro.
fonte