Passando um objeto para um método que o altera, é um padrão (anti-) comum?

17

Estou lendo sobre cheiros de códigos comuns no livro de Refatoração de Martin Fowler . Nesse contexto, eu estava pensando em um padrão que estou vendo em uma base de código, e se alguém poderia objetivamente considerá-lo um anti-padrão.

O padrão é aquele em que um objeto é passado como argumento para um ou mais métodos, todos os quais alteram o estado do objeto, mas nenhum deles retorna o objeto. Portanto, ele conta com o passe por natureza de referência (neste caso) C # / .NET.

var something = new Thing();
// ...
Foo(something);
int result = Bar(something, 42);
Baz(something);

Descobri que (especialmente quando os métodos não são nomeados adequadamente), preciso investigar esses métodos para entender se o estado do objeto foi alterado. Isso torna a compreensão do código mais complexa, pois preciso rastrear vários níveis da pilha de chamadas.

Eu gostaria de propor a melhoria desse código para retornar outro objeto (clonado) com o novo estado ou qualquer coisa necessária para alterar o objeto no site de chamada.

var something1 =  new Thing();
// ...

// Let's return a new instance of Thing
var something2 = Foo(something1);

// Let's use out param to 'return' other info about the operation
int result;
var something3 = Bar(something2, out result);

// If necessary, let's capture and make explicit complex changes
var changes = Baz(something3)
something3.Apply(changes);

Para mim, parece que o primeiro padrão é escolhido nas suposições

  • que é menos trabalhoso ou requer menos linhas de código
  • que nos permite mudar o objeto e retornar alguma outra informação
  • que é mais eficiente, pois temos menos instâncias de Thing.

Ilustro uma alternativa, mas para propor, é preciso ter argumentos contra a solução original. Quais argumentos, se houver, podem ser apresentados para afirmar que a solução original é um antipadrão?

E o que há de errado com minha solução alternativa?

Michiel van Oosterhout
fonte
1
Este é um efeito colateral #
Dave Hillier
1
@DaveHillier Obrigado, eu estava familiarizado com o termo, mas não havia feito a conexão.
Michiel van Oosterhout

Respostas:

9

Sim, a solução original é um antipadrão pelos motivos que você descreve: dificulta o raciocínio sobre o que está acontecendo, o objeto não é responsável por seu próprio estado / implementação (quebra do encapsulamento). Eu também acrescentaria que todas essas mudanças de estado são contratos implícitos do método, tornando esse método frágil diante das mudanças nos requisitos.

Dito isto, sua solução tem algumas desvantagens, a mais óbvia delas é que a clonagem de objetos não é boa. Pode ser lento para objetos grandes. Isso pode levar a erros onde outras partes do código se apegam às referências antigas (o que provavelmente ocorre na base de código que você descreve). Tornar esses objetos explicitamente imutáveis ​​resolve pelo menos alguns desses problemas, mas é uma mudança mais drástica.

A menos que os objetos sejam pequenos e de certa forma transitórios (o que os torna bons candidatos à imutabilidade), eu estaria inclinado a simplesmente mover mais da transição de estado para os próprios objetos. Isso permite ocultar os detalhes de implementação dessas transições e definir requisitos mais fortes sobre quem / o quê / onde essas transições de estado ocorrem.

Telastyn
fonte
1
Por exemplo, se eu tiver um objeto "Arquivo", não tentarei mover nenhum método de alteração de estado para esse objeto - isso violaria o SRP. Isso é válido mesmo quando você tem suas próprias classes em vez de uma classe de biblioteca como "Arquivo" - colocar toda lógica de transição de estado na classe do objeto não faz muito sentido.
Doc Brown
@ Tetastyn Eu sei que essa é uma resposta antiga, mas estou tendo problemas para visualizar sua sugestão no último parágrafo em termos concretos. Você poderia elaborar ou dar um exemplo?
AaronLS
@AaronLS - Em vez de Bar(something)(e modificar o estado de something), faça Barum membro do somethingtipo. something.Bar(42)é mais provável que sofra mutação something, além de permitir que você use ferramentas OO (estado privado, interfaces, etc.) para proteger somethingo estado
Telastyn
14

quando os métodos não são nomeados adequadamente

Na verdade, esse é o verdadeiro cheiro do código. Se você tiver um objeto mutável, ele fornece métodos para alterar seu estado. Se você tem uma chamada para esse método incorporado em uma tarefa de mais algumas instruções, não há problema em refatorar essa tarefa para um método próprio - o que o deixa exatamente na situação descrita. Mas se você não tem nomes de métodos como Fooe Bar, mas métodos que deixam claro que eles alteram o objeto, não vejo problema aqui. Imagine

void AddMessageToLog(Logger logger, string msg)
{
    //...
}

ou

void StripInvalidCharsFromName(Person p)
{
// ...
}

ou

void AddValueToRepo(Repository repo,int val)
{
// ...
}

ou

void TransferMoneyBetweenAccounts(Account source, Account destination, decimal amount)
{
// ...
}

ou algo assim - não vejo aqui nenhuma razão para retornar um objeto clonado para esses métodos, e também não há razão para analisar sua implementação para entender que eles mudarão o estado do objeto passado.

Se você não quiser efeitos colaterais, torne seus objetos imutáveis, ele aplicará métodos como os anteriores para retornar um objeto alterado (clonado) sem alterar o original.

Doc Brown
fonte
Você está certo, a refatoração do método de renomeação pode melhorar a situação, tornando claros os efeitos colaterais. Pode tornar-se difícil, se as modificações forem tais que um nome de método conciso não seja possível.
Michiel van Oosterhout
2
@michielvoo: se não for possível nomear um método consenso, seu método agrupa as coisas erradas em vez de criar uma abstração funcional para a tarefa que realiza (e isso é verdade com ou sem efeitos colaterais).
Doc Brown
4

Sim, consulte http://codebetter.com/matthewpodwysocki/2008/04/30/side-effecting-functions-are-code-smells/ para obter um dos muitos exemplos de pessoas apontando que efeitos colaterais inesperados são ruins.

Em geral, o princípio fundamental é que o software é construído em camadas, e cada camada deve apresentar a abstração mais limpa possível para a próxima. E uma abstração limpa é aquela em que você deve ter o mínimo possível em mente para usá-la. Isso se chama modularidade e se aplica a tudo, desde funções únicas a protocolos em rede.

btilly
fonte
Eu caracterizaria o que o OP está descrevendo como "efeitos colaterais esperados". Por exemplo, um delegado pode ser passado para um mecanismo de algum tipo que opera em cada elemento de uma lista. Isso é basicamente o que ForEach<T>faz.
21713 Robert Harvey
@RobertHarvey As reclamações sobre os métodos não serem nomeados corretamente e sobre a necessidade de ler o código para descobrir os efeitos colaterais, os tornam definitivamente não esperados.
btilly
Eu concedo isso a você. Mas o corolário é que um método documentado nomeado corretamente com efeitos esperados no site pode não ser um antipadrão, afinal.
Robert Harvey
@RobertHarvey Eu concordo. A chave é que efeitos colaterais significativos são muito importantes para serem conhecidos e precisam ser documentados com cuidado (de preferência no nome do método).
btilly
Eu diria que é uma mistura de efeitos colaterais inesperados e não óbvios. Obrigado pelo link.
Michiel van Oosterhout
3

Antes de tudo, isso não depende da "natureza de referência por", depende dos objetos serem tipos de referência mutáveis. Em linguagens não funcionais, esse quase sempre será o caso.

Em segundo lugar, se isso é um problema ou não, depende tanto do objeto quanto da rigidez das alterações nos diferentes procedimentos - se você não conseguir fazer uma alteração no Foo e causar uma falha na barra, será um problema. Não é necessariamente um cheiro de código, mas é um problema com Foo ou Bar ou Something (provavelmente Bar como deveria estar verificando sua entrada, mas poderia ser Algo sendo colocado em um estado inválido que deveria estar impedindo).

Eu não diria que sobe para o nível de um antipadrão, mas algo para estar ciente.

jmoreno
fonte
2

Eu diria que há pouca diferença entre A.Do(Something)modificar somethinge something.Do()modificar something. Em qualquer um dos casos, deve ficar claro o nome do método invocado que somethingserá modificado. Se não estiver claro o nome do método, independentemente de somethingser um parâmetro thisou parte do ambiente, ele não deverá ser modificado.

svidgen
fonte
1

Eu acho que é bom mudar o estado do objeto em alguns cenários. Por exemplo, tenho uma lista de usuários e desejo aplicar filtros diferentes à lista antes de devolvê-la ao cliente.

var users = Dependency.Resolve<IGetUsersQuery>().GetAll();

var excludeAdminUsersFilter = new ExcludeAdminUsersFilter();
var filterByAnotherCriteria = new AnotherCriteriaFilter();

excludeAdminUsersFilter.Apply(users);
filterByAnotherCriteria.Apply(users); 

E sim, você pode tornar isso bonito movendo a filtragem para outro método, para acabar com algo nas linhas de:

var users = Dependency.Resolve<IGetUsersQuery>().GetAll();
Filter(users);

Onde Filter(users)seria executado acima dos filtros.

Não me lembro exatamente onde me deparei com isso antes, mas acho que isso foi chamado de pipeline de filtragem.

CodeART
fonte
0

Não tenho certeza se a nova solução proposta (de copiar objetos) é um padrão. O problema, como você apontou, é a má nomenclatura de funções.

Digamos que eu escreva uma operação matemática complexa como uma função f () . I documentar que f () é uma função que mapeia NXNpara N, eo algoritmo por trás dele. Se a função for nomeada inadequadamente, e não documentada, e não tiver casos de teste acompanhantes, você precisará entender o código; nesse caso, o código será inútil.

Sobre sua solução, algumas observações:

  • Os aplicativos são projetados sob diferentes aspectos: quando um objeto é usado apenas para armazenar valores ou é passado através dos limites dos componentes, é aconselhável alterar as entranhas do objeto externamente, em vez de preenchê-lo com detalhes de como alterar.
  • A clonagem de objetos leva a inchaço dos requisitos de memória e, em muitos casos, à existência de objetos equivalentes em estados incompatíveis ( Xtornou-se Ydepois f(), mas Xna verdade é Y) e possivelmente inconsistência temporal.

O problema que você está tentando resolver é válido; no entanto, mesmo com a enorme superengenharia, o problema é contornado, não resolvido.

CMR
fonte
2
Essa seria uma resposta melhor se você relacionasse sua observação à pergunta do OP. Como é, é mais um comentário do que uma resposta.
Robert Harvey
1
@RobertHarvey +1, boa observação, eu concordo, irá editá-lo.
CMR