Passare un oggetto in un metodo che cambia l'oggetto, è un modello (anti-) comune?

Sì, la soluzione originale è un anti-pattern per i motivi che descrivi: rende difficile ragionare su ciò che sta accadendo, l'oggetto non è responsabile per il suo stato / implementazione (rottura incapsulamento). Vorrei anche aggiungere che tutti questi cambiamenti di stato sono contratti impliciti del metodo, rendendo questo metodo fragile di fronte ai mutevoli requisiti.

Detto questo, la tua soluzione ha alcuni dei suoi lati negativi, il più ovvio dei quali è che gli oggetti di clonazione non sono grandiosi. Può essere lento per oggetti di grandi dimensioni. Può portare a errori in cui altre parti del codice si aggrappano ai vecchi riferimenti (che è probabilmente il caso nella base di codice che descrivi). Rendere questi oggetti esplicitamente immutabili risolve almeno alcuni di questi problemi, ma è un cambiamento più drastico.

A meno che gli oggetti siano piccoli e in qualche modo transitori (che li rendono buoni candidati per l'immutabilità), sarei incline a spostare semplicemente più della transizione di stato negli oggetti stessi. Ciò ti consente di nascondere i dettagli di implementazione di queste transizioni e di stabilire requisiti più rigidi su chi / cosa / dove si verificano tali transizioni di stato.

when methods are not named appropriately

In realtà è il vero odore del codice. Se si dispone di un oggetto mutabile, fornisce metodi per cambiare il suo stato. Se si dispone di una chiamata a un tale metodo incorporato in un'attività di alcune altre istruzioni, è corretto ricondurre tale attività a un metodo a sé stante, che ti lascia esattamente la situazione descritta. Ma se non hai nomi di metodi come Foo e Bar , ma metodi che rendono chiaro che cambiano l'oggetto, non vedo qui un problema. Pensa

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

o

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

o

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

o

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

o qualcosa del genere - Qui non vedo alcun motivo per restituire un oggetto clonato per quei metodi, e non c'è nemmeno motivo di esaminare la loro implementazione per capire che cambieranno lo stato dell'oggetto passato.

Se non vuoi effetti secondari, rendi i tuoi oggetti immutabili, applicherà metodi come quelli precedenti per restituire un oggetto modificato (clonato) senza cambiare quello originale.

Sì, vedi link per uno dei tanti esempi di persone che sottolineano che gli effetti collaterali inaspettati sono negativi.

In generale il principio fondamentale è che il software è costruito a strati, e ogni livello dovrebbe presentare l'astrazione più pulita possibile alla successiva. E un'astrazione pulita è quella in cui devi tenere il meno possibile in mente per usarlo. Si chiama modularità e si applica a tutto, dalle singole funzioni ai protocolli di rete.

Prima di tutto, questo non dipende dalla "natura di riferimento per passaggio di" dipende da oggetti che sono tipi di riferimento mutabili. Nei linguaggi non funzionali è quasi sempre il caso.

In secondo luogo, se questo è un problema o meno, dipende sia dall'oggetto sia dalla forza con cui i cambiamenti nelle diverse procedure sono legati tra loro - se non si riesce a fare un cambiamento in Foo e ciò causa il blocco di Bar, allora è un problema. Non necessariamente un odore di codice, ma è un problema con Foo o Bar o Something (probabilmente Bar come dovrebbe essere il suo input, ma potrebbe essere Qualcosa essere messo in uno stato non valido che dovrebbe impedire).

Non direi che sale al livello di un anti-pattern, ma piuttosto qualcosa da tenere presente.

Direi che c'è poca differenza tra A.Do(Something) che modifica something e something.Do() che modifica something . Nel caso uno , dovrebbe essere chiaro dal nome del metodo richiamato che something sarà modificato. Se non è chiaro dal nome del metodo, indipendentemente dal fatto che something sia un parametro, this o parte dell'ambiente, non dovrebbe essere modificato.

Penso che sia bello cambiare lo stato dell'oggetto in alcuni scenari. Ad esempio, ho un elenco di utenti e voglio applicare diversi filtri all'elenco prima di restituirlo al client.

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

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

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

E sì, puoi renderlo bello spostando il filtraggio in un altro metodo, così finirai con qualcosa sulle linee di:

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

Dove Filter(users) verrebbe eseguito sopra i filtri.

Non ricordo esattamente dove mi sono imbattuto prima, ma penso che sia stato definito pipeline filtrante.

Non sono sicuro che la nuova soluzione proposta (di copia degli oggetti) sia un modello. Il problema, come hai sottolineato, è la cattiva nomenclatura delle funzioni.

Diciamo che scrivo un'operazione matematica complessa come una funzione f () . Documento che f () è una funzione che mappa NXN in N e l'algoritmo dietro di esso. Se la funzione è denominata in modo inappropriato e non documentata, e non ha casi di test di accompagnamento, e si dovrà capire il codice, nel qual caso il codice è inutile.

Informazioni sulla soluzione, alcune osservazioni:

• Le applicazioni sono progettate da diversi aspetti: quando un oggetto viene utilizzato solo per contenere valori, o viene passato attraverso i limiti dei componenti, è consigliabile modificare esternamente le interiora dell'oggetto anziché riempirlo con i dettagli su come cambiare.
• La clonazione di oggetti porta a requisiti di memoria eccessivi e in molti casi porta all'esistenza di oggetti equivalenti in stati incompatibili ( X diventato Y dopo f() , ma X è in realtà Y ,) e eventualmente incoerenza temporale.

Il problema che stai cercando di affrontare è valido; tuttavia, anche con un enorme lavoro di rifinitura, il problema viene aggirato, non risolto.