Handelt es sich bei der Übergabe eines Objekts an eine Methode, die das Objekt ändert, um ein allgemeines (Anti-) Muster?

17

Ich lese über übliche Codegerüche in Martin Fowlers Refactoring-Buch . In diesem Zusammenhang habe ich mich über ein Muster gewundert, das ich in einer Codebasis sehe, und ob man es objektiv als Anti-Muster betrachten kann.

Das Muster ist eines, bei dem ein Objekt als Argument an eine oder mehrere Methoden übergeben wird, die alle den Zustand des Objekts ändern, von denen jedoch keine das Objekt zurückgeben. Es basiert also auf der Referenzübergabe von (in diesem Fall) C # /. NET.

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

Ich finde, dass (insbesondere wenn Methoden nicht entsprechend benannt sind) ich solche Methoden untersuchen muss, um zu verstehen, ob sich der Zustand des Objekts geändert hat. Dadurch wird das Codeverständnis komplexer, da ich mehrere Ebenen des Aufrufstapels nachverfolgen muss.

Ich möchte vorschlagen, einen solchen Code zu verbessern, um ein anderes (geklontes) Objekt mit dem neuen Status zurückzugeben, oder alles, was zum Ändern des Objekts am Aufrufstandort erforderlich ist.

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);

Mir scheint, dass das erste Muster anhand der Annahmen gewählt wird

  • dass es weniger Arbeit ist oder weniger Codezeilen erfordert
  • dass es erlaubt uns , das Objekt sowohl Veränderung, und einige andere Information zurückgeben
  • dass es effizienter ist, da wir weniger Instanzen von haben Thing.

Ich illustriere eine Alternative, aber um sie vorzuschlagen, muss man Argumente gegen die ursprüngliche Lösung haben. Was, wenn überhaupt, kann dafür argumentiert werden, dass die ursprüngliche Lösung ein Anti-Pattern ist?

Und was stimmt mit meiner alternativen Lösung nicht?

Michiel van Oosterhout
quelle
1
Dies ist ein Nebeneffekt
Dave Hillier
1
@ DaveHillier Danke, ich war mit dem Begriff vertraut, hatte aber die Verbindung nicht hergestellt.
Michiel van Oosterhout

Antworten:

9

Ja, die ursprüngliche Lösung ist aus den von Ihnen beschriebenen Gründen ein Anti-Pattern: Es macht es schwierig zu überlegen, was vor sich geht, und das Objekt ist nicht für seinen eigenen Zustand / seine eigene Implementierung verantwortlich (Kapselung aufheben). Ich möchte auch hinzufügen, dass alle diese Zustandsänderungen implizite Verträge der Methode sind, was diese Methode angesichts sich ändernder Anforderungen zerbrechlich macht.

Das heißt, Ihre Lösung hat einige Nachteile. Das offensichtlichste davon ist, dass das Klonen von Objekten nicht großartig ist. Bei großen Objekten kann dies langsam sein. Dies kann zu Fehlern führen, wenn andere Teile des Codes an den alten Verweisen festhalten (was in der von Ihnen beschriebenen Codebasis wahrscheinlich der Fall ist). Indem Sie diese Objekte explizit unveränderlich machen, lösen Sie zumindest einige dieser Probleme, stellen jedoch eine drastischere Änderung dar.

Wenn die Objekte nicht klein und vorübergehend sind (was sie zu guten Kandidaten für die Unveränderlichkeit macht), wäre ich geneigt, einfach mehr vom Zustandsübergang in die Objekte selbst zu verlagern. Auf diese Weise können Sie die Implementierungsdetails dieser Übergänge ausblenden und strengere Anforderungen an wen / was / wo diese Statusübergänge auftreten.

Telastyn
quelle
1
Wenn ich zum Beispiel ein "Datei" -Objekt habe, würde ich nicht versuchen, eine Methode zum Ändern des Status in dieses Objekt zu verschieben - dies würde die SRP verletzen. Dies gilt auch dann, wenn Sie anstelle einer Bibliotheksklasse wie "File" eigene Klassen haben - eine Zuordnung jeder Zustandsübergangslogik zur Klasse des Objekts ist nicht wirklich sinnvoll.
Doc Brown
@Tetastyn Ich weiß, dass dies eine alte Antwort ist, aber ich habe Probleme, Ihren Vorschlag im letzten Absatz konkret zu visualisieren. Könnten Sie etwas näher erläutern oder ein Beispiel nennen?
AaronLS
@AaronLS - Erstellen Sie anstelle von Bar(something)(und Ändern des Status von something) Barein Mitglied vom somethingTyp. something.Bar(42)ist wahrscheinlicher zu mutieren something, während Sie auch OO-Tools (privater Status, Schnittstellen usw.) zum Schutz somethingdes Status verwenden können
Telastyn
14

wenn Methoden nicht richtig benannt sind

Eigentlich das ist der eigentliche Code Geruch. Wenn Sie über ein veränderbares Objekt verfügen, werden Methoden zum Ändern seines Status bereitgestellt. Wenn Sie einen Aufruf einer solchen Methode haben, die in eine Task mit mehreren Anweisungen eingebettet ist, ist es in Ordnung, diese Task in eine eigene Methode umzuwandeln - was Sie in der genau beschriebenen Situation zurücklässt. Aber wenn Sie keine Methodennamen wie Foound haben Bar, sondern Methoden, die deutlich machen, dass sie das Objekt ändern, sehe ich hier kein Problem. Denk an

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

oder

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

oder

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

oder

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

oder so ähnlich - Ich sehe hier keinen Grund, ein geklontes Objekt für diese Methoden zurückzugeben, und es gibt auch keinen Grund, in ihre Implementierung zu schauen, um zu verstehen, dass sie den Status des übergebenen Objekts ändern.

Wenn Sie keine Nebenwirkungen wünschen und Ihre Objekte unveränderlich machen möchten, werden Methoden wie die oben genannten erzwungen, um ein geändertes (geklontes) Objekt zurückzugeben, ohne das ursprüngliche zu ändern.

Doc Brown
quelle
Sie haben Recht, die Umbenennungsmethode Refactoring kann die Situation verbessern, indem Nebenwirkungen deutlich gemacht werden. Es kann jedoch schwierig werden, wenn die Änderungen so vorgenommen werden, dass ein genauer Methodenname nicht möglich ist.
Michiel van Oosterhout
2
@michielvoo: Wenn eine genaue Benennung von Methoden nicht möglich zu sein scheint, gruppiert Ihre Methode die falschen Dinge, anstatt eine funktionale Abstraktion für die Aufgabe zu erstellen, die sie ausführt (und das gilt mit oder ohne Nebenwirkungen).
Doc Brown
4

Ja, siehe http://codebetter.com/matthewpodwysocki/2008/04/30/side-effecting-functions-are-code-smells/ für eines von vielen Beispielen von Personen, die darauf hinweisen, dass unerwartete Nebenwirkungen schlimm sind.

Im Allgemeinen besteht das Grundprinzip darin, dass Software in Schichten aufgebaut ist und jede Schicht der nächsten eine möglichst saubere Abstraktion bieten sollte. Und eine saubere Abstraktion ist eine, bei der man so wenig wie möglich beachten muss, um sie zu verwenden. Das nennt sich Modularität und gilt von einzelnen Funktionen bis hin zu vernetzten Protokollen.

btilly
quelle
Ich würde charakterisieren, was das OP als "erwartete Nebenwirkungen" beschreibt. Ein Delegat, den Sie beispielsweise an eine Engine übergeben können, die auf jedes Element in einer Liste angewendet wird. Das ist im Grunde, was ForEach<T>macht.
Robert Harvey
@RobertHarvey Die Beschwerden, dass Methoden nicht richtig benannt wurden und dass Code gelesen werden musste, um die Nebenwirkungen herauszufinden, lassen sie definitiv keine Nebenwirkungen erwarten.
btilly
Ich werde dir das gewähren. Die Konsequenz ist jedoch, dass eine ordnungsgemäß benannte dokumentierte Methode mit erwarteten Site-Effekten möglicherweise doch kein Anti-Pattern ist.
Robert Harvey
@RobertHarvey Ich stimme zu. Der Schlüssel ist, dass signifikante Nebenwirkungen sehr wichtig sind und sorgfältig dokumentiert werden müssen (vorzugsweise im Namen der Methode).
btilly
Ich würde sagen, es ist eine Mischung aus unerwarteten und nicht offensichtlichen Nebenwirkungen. Danke für den Link.
Michiel van Oosterhout
3

Erstens hängt dies nicht von der Art der Referenzübergabe ab, sondern davon, ob Objekte veränderbare Referenztypen sind. In nicht funktionalen Sprachen wird das fast immer der Fall sein.

Zweitens, ob dies ein Problem ist oder nicht, hängt sowohl vom Objekt als auch davon ab, wie eng die Änderungen in den verschiedenen Prozeduren miteinander verknüpft sind. Wenn Sie in Foo keine Änderung vornehmen und dies dazu führt, dass Bar abstürzt, ist dies ein Problem. Nicht unbedingt ein Codegeruch, aber es ist ein Problem mit Foo oder Bar oder Something (wahrscheinlich Bar, da es seine Eingabe überprüfen sollte, aber es könnte sein, dass Something in einen ungültigen Zustand versetzt wird, den es verhindern sollte).

Ich würde nicht sagen, dass es auf das Niveau eines Anti-Musters steigt, sondern etwas, das man beachten muss.

jmoreno
quelle
2

Ich würde behaupten, dass es kaum einen Unterschied zwischen A.Do(Something)Modifizieren somethingund something.Do()Modifizieren gibt something. In beiden Fällen sollte es aus dem Namen der aufgerufenen Methode ersichtlich somethingsein, die geändert wird. Wenn der Methodenname nicht eindeutig ist, unabhängig davon, ob somethinges sich um einen Parameter thisoder einen Teil der Umgebung handelt, sollte er nicht geändert werden.

Svidgen
quelle
1

Ich denke, es ist in einigen Szenarien in Ordnung, den Status des Objekts zu ändern. Ich habe zum Beispiel eine Liste von Benutzern und möchte verschiedene Filter auf die Liste anwenden, bevor ich sie an den Client zurückschicke.

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

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

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

Und ja, Sie können dies hübsch machen, indem Sie die Filterung in eine andere Methode verschieben. Am Ende erhalten Sie also Folgendes:

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

Wo Filter(users)würde oben Filter ausführen.

Ich kann mich nicht erinnern, wo genau ich darauf gestoßen bin, aber ich glaube, es wurde als Filterpipeline bezeichnet.

CodeART
quelle
0

Ich bin nicht sicher, ob die vorgeschlagene neue Lösung (zum Kopieren von Objekten) ein Muster ist. Das Problem ist, wie Sie betont haben, die schlechte Nomenklatur der Funktionen.

Nehmen wir an, ich schreibe eine komplexe mathematische Operation als Funktion f () . Ich dokumentiere , dass f () ist eine Funktion , die Karten NXNauf N, und der Algorithmus dahinter. Wenn die Funktion einen falschen Namen trägt und nicht dokumentiert ist und keine zugehörigen Testfälle enthält und Sie den Code verstehen müssen, ist der Code in diesem Fall unbrauchbar.

Zu Ihrer Lösung einige Beobachtungen:

  • Anwendungen werden unter verschiedenen Gesichtspunkten entworfen: Wenn ein Objekt nur zum Speichern von Werten verwendet wird oder über Komponentengrenzen hinweg übergeben wird, ist es ratsam, die Innereien des Objekts extern zu ändern, anstatt sie mit Details zum Ändern zu füllen.
  • Cloning - Objekte führen Speicheranforderungen zu Blähungen, und in vielen Fällen führen die Existenz von äquivalenten Objekten in unvereinbar Staaten ( Xwurde Ynach f(), sondern Xist tatsächlich Y,) und möglicherweise zeitliche Inkonsistenz.

Das Problem, das Sie ansprechen möchten, ist gültig. Doch selbst mit enormen Überentwicklungen wird das Problem umgangen und nicht gelöst.

CMR
quelle
2
Dies wäre eine bessere Antwort, wenn Sie Ihre Beobachtung mit der Frage des OP in Verbindung bringen würden. So wie es ist, ist es eher ein Kommentar als eine Antwort.
Robert Harvey
1
@RobertHarvey +1, gute Beobachtung, ich stimme zu, werde es bearbeiten.
CMR