Ist das Ändern eines eingehenden Parameters ein Gegenmuster? [geschlossen]

60

Ich programmiere in Java und mache Konverter immer so:

public OtherObject MyObject2OtherObject(MyObject mo){
    ... Do the conversion
    return otherObject;
}

Am neuen Arbeitsplatz lautet das Muster:

public void MyObject2OtherObject(MyObject mo, OtherObject oo){
    ... Do the conversion
}

Für mich ist es ein bisschen stinkend, da ich mich daran gewöhnt habe, die eingehenden Parameter nicht zu ändern. Handelt es sich bei dieser Änderung des eingehenden Parameters um ein Antimuster oder ist dies in Ordnung? Hat es einige gravierende Nachteile?

CsBalazsHungary
quelle
4
Die richtige Antwort hierfür wird sprachspezifisch sein, da die Parameter, bei denen Werte übergeben werden, sie effektiv in lokale Variablen umwandeln.
Blrfl
1
Das zweite Muster wird manchmal als Effizienzmaß für die Wiederverwendung von Objekten verwendet. Angenommen, es oohandelt sich um ein Objekt, das an die Methode übergeben wird, und nicht um einen Zeiger, der auf ein neues Objekt gesetzt wird. Ist das hier der Fall? Wenn dies Java ist, ist es wahrscheinlich, wenn sein C ++ es nicht sein könnte
Richard Tingle
3
Das sieht nach vorzeitiger Optimierung aus. Ich empfehle, dass Sie das erste Formular im Allgemeinen verwenden, aber wenn Sie auf einen Leistungsengpass stoßen, können Sie das zweite Formular mit einem Kommentar verwenden, um ihn zu begründen . Ein Kommentar würde Sie und andere daran hindern, sich diesen Code anzusehen und dieselbe Frage immer wieder auftauchen zu lassen.
Keen
2
Das zweite Muster ist nützlich, wenn die Funktion ebenfalls einen Wert zurückgibt, z. B. Erfolg / Misserfolg. Aber es ist nichts wirklich "falsch" daran. Es hängt wirklich davon ab, wo Sie das Objekt erstellen möchten.
GrandmasterB
12
Ich würde Funktionen, die diese beiden unterschiedlichen Muster implementieren, nicht auf dieselbe Weise benennen.
Casey

Antworten:

70

Es ist kein Gegenmuster, es ist eine schlechte Praxis.

Der Unterschied zwischen einem Antimuster und einer schlechten Praxis ist hier: Anti-Muster-Definition .

Der neue Arbeitsplatzstil, den Sie zeigen, ist nach dem Clean Code von Uncle Bob eine schlechte Praxis , egal ob es sich um eine Zeit vor oder nach dem OOP handelt.

Argumente werden am natürlichsten als Eingaben in eine Funktion interpretiert.

Alles, was Sie dazu zwingt, die Funktionssignatur zu überprüfen, entspricht einem Double-Take. Es ist eine kognitive Unterbrechung und sollte vermieden werden. In den Tagen vor der objektorientierten Programmierung war es manchmal erforderlich, Ausgabeargumente zu haben. In OO-Sprachen entfällt jedoch ein Großteil des Bedarfs an Ausgabeargumenten

Tulains Córdova
quelle
7
Ich verstehe nicht, warum die Definition, mit der Sie verlinkt haben, ausschließt, dass dies als Anti-Muster angesehen wird.
Samuel Edwin Ward
7
Ich nehme an, es liegt daran, dass die Begründung, dass "wir CPU / mem sparen, indem wir kein neues Objekt erstellen, stattdessen eines, das wir haben, recyceln und als Argument übergeben" für jeden mit ernsthaftem OOP-Hintergrund so offensichtlich falsch ist, dass dies der Fall sein könnte Es gibt also keine Möglichkeit, es als Anti-Muster zu betrachten ...
vaxquis
1
Was ist mit dem Fall, wenn ein Eingabeparameter eine große Sammlung von Objekten ist, die geändert werden müssen?
Steve Chambers
Obwohl ich auch Option 1 bevorzuge, was OtherObjectist , wenn es sich um eine Schnittstelle handelt? Nur der Anrufer weiß (hoffentlich), welchen konkreten Typ er will.
user949300
@SteveChambers Ich denke, in diesem Fall ist das Design der Klasse oder Methode schlecht und sollte überarbeitet werden, um dies zu vermeiden.
piotr.wittchen
16

Zitat von Robert C. Martins berühmtem Buch "Clean Code":

Ausgabeargumente sollten vermieden werden

Funktionen sollten eine kleine Anzahl von Argumenten haben

Das zweite Muster verletzt beide Regeln, insbesondere das "Ausgabeargument". In diesem Sinne ist es schlimmer als das erste Muster.

claasz
quelle
1
Ich würde sagen, es verstößt gegen den ersten, viele Argumente bedeuten chaotischen Code, aber ich denke, zwei Argumente sind völlig in Ordnung. Ich denke, dass eine Regel für sauberen Code bedeutet, dass Sie, wenn Sie 5 oder 6 eingehende Daten benötigen, zu viel in einer einzelnen Methode tun möchten. Sie sollten also überarbeiten, um die Lesbarkeit des Codes und den OOP-Bereich zu verbessern.
CsBalazsHungary
2
Ich vermute jedenfalls, dass dies kein strenges Gesetz ist, sondern ein starker Vorschlag. Ich weiß, dass es mit primitiven Typen nicht funktioniert, wenn es sich um ein weit verbreitetes Muster in einem Projekt handelt, würde ein Junior Ideen dazu bringen, ein Int zu übergeben, und erwarten, dass es wie Objekte geändert wird.
CsBalazsHungary
27
Unabhängig davon, wie korrekt Clean Code sein mag, halte ich dies nicht für eine wertvolle Antwort, ohne zu erklären, warum dies vermieden werden sollte. Es sind keine Gebote, die auf eine Steintafel fallen. Verständnis ist wichtig. Diese Antwort könnte verbessert werden, indem eine Zusammenfassung der Argumentation in dem Buch und ein Kapitelverweis
Daenyth,
3
Wenn jemand es in ein Buch geschrieben hat, muss es ein guter Rat für jede denkbare Situation sein. Kein Grund zum Nachdenken.
Casey
2
@emodendroket Aber Alter, es ist der Typ!
Pierre Arlaud
15

Das zweite Idiom kann schneller sein, da der Aufrufer eine Variable über eine lange Schleife wiederverwenden kann, anstatt bei jeder Iteration eine neue Instanz zu erstellen.

Ich würde es im Allgemeinen nicht verwenden, aber zB. In der Spielprogrammierung hat es seinen Platz. Wenn Sie sich beispielsweise die vielen Operationen von JavaMonkey Vector3f ansehen, können Sie Instanzen übergeben, die geändert und als Ergebnis zurückgegeben werden sollen.

user470365
quelle
12
Nun, ich kann zustimmen, ob dies der Engpass ist, aber wo immer ich gearbeitet habe, sind die Engpässe normalerweise Algorithmen mit geringer Effizienz, nicht das Klonen oder Erstellen von Objekten. Ich weiß weniger über Spieleentwicklung.
CsBalazsHungary
4
@CsBalazsHungary Ich glaube, dass die Probleme, die bei der Objekterstellung auftreten, in der Regel mit der Speicherzuweisung und der Speicherbereinigung zusammenhängen. Dies wäre wahrscheinlich die nächste Stufe von Engpässen nach der algorithmischen Komplexität (insbesondere in einer Umgebung mit begrenztem Speicher, z. B. Smartphones und dergleichen). .
JAB
JMonkey ist auch der Ort, an dem ich dies oft gesehen habe, da es oft leistungskritisch ist. Ich habe noch nie gehört, dass es "JavaMonkey" heißt
Richard Tingle
3
@JAB: Der GC der JVM ist speziell für den Fall von kurzlebigen Objekten abgestimmt. Große Mengen von sehr kurzlebigen Objekten sind trivial zu sammeln, und in vielen Fällen kann Ihre gesamte kurzlebige Generation mit einer einzigen Zeigerbewegung gesammelt werden.
Phoshi
2
Tatsächlich ist ein Objekt , das erstellt werden muss, nicht leistungswirksam. Wenn ein Codeblock stark auf Geschwindigkeit optimiert werden muss, sollten Sie stattdessen Primitive und native Arrays verwenden. Aus diesem Grund denke ich, dass die Aussage in dieser Antwort nicht zutrifft.
Vaxquis
10

Ich denke nicht, dass dies zwei gleichwertige Codeteile sind. Im ersten Fall müssen Sie die erstellen otherObject. Sie können die vorhandene Instanz in der zweiten ändern. Beide haben ihren Nutzen. Code-Geruch würde einander vorziehen.

Euphorisch
quelle
Ich weiß, dass sie nicht gleichwertig sind. Sie haben recht, wir halten an diesen Dingen fest, es würde also einen Unterschied machen. Sie sagen also, dass keines von ihnen Antimuster ist, es ist nur eine Frage des Anwendungsfalls?
CsBalazsHungary
@CsBalazsHungary würde ich ja sagen. Gemessen an dem kleinen Code, den Sie bereitgestellt haben.
Euphoric
Ich vermute, es wird ein ernstes Problem, wenn Sie einen eingehenden primitiven Parameter haben, der natürlich nicht aktualisiert wird.
CsBalazsHungary
1
@CsBalazsHungary Im Falle eines primitiven Arguments würde die Funktion nicht einmal funktionieren. Sie haben also ein schlimmeres Problem als das Ändern von Argumenten.
Euphoric
Ich würde sagen, dass ein Junior in die Falle tappen würde, einen primitiven Typ zu einem Ausgabeparameter zu machen. Daher würde ich sagen, der erste Konverter sollte nach Möglichkeit bevorzugt werden.
CsBalazsHungary
7

Es kommt wirklich auf die Sprache an.

In Java kann die zweite Form ein Anti-Pattern sein, aber einige Sprachen behandeln die Parameterübergabe unterschiedlich. In Ada und VHDL beispielsweise anstelle von Pass Wert oder als Referenz, können Parameter haben Modi in, outoder in out.

Es ist ein Fehler, einen inParameter zu ändern oder zu lesen out, aber alle Änderungen an einem in outParameter werden an den Aufrufer zurückgegeben.

Also die beiden Formen in Ada (diese sind auch legale VHDL) sind

function MyObject2OtherObject(mo : in MyObject) return OtherObject is
begin
    ... Do the conversion
    return otherObject;
end MyObject2OtherObject;

und

procedure MyObject2OtherObject(mo : in MyObject; oo : out OtherObject) is
begin
    ... Do the conversion
    oo := ... the conversion result;
end MyObject2OtherObject;

Beide haben ihren Nutzen; Eine Prozedur kann mehrere Werte in mehreren OutParametern zurückgeben, während eine Funktion nur ein einziges Ergebnis zurückgeben kann. Da der Zweck des zweiten Parameters in der Verfahrenserklärung eindeutig angegeben ist, kann gegen dieses Formular kein Einwand erhoben werden. Ich bevorzuge die Funktion für die Lesbarkeit. Es wird jedoch Fälle geben, in denen die Prozedur besser ist, z. B. wenn der Aufrufer das Objekt bereits erstellt hat.

Brian Drummond
quelle
3

Es scheint in der Tat stinkend, und ohne mehr Kontext ist es unmöglich, sicher zu sagen. Dies könnte zwei Gründe haben, obwohl es für beide Alternativen gibt.

Erstens ist dies eine übersichtliche Methode, um eine teilweise Konvertierung durchzuführen oder das Ergebnis mit Standardwerten zu belassen, wenn die Konvertierung fehlschlägt. Das heißt, Sie könnten dies haben:

public void ConvertFoo(Foo from, Foo to) {
    if (can't convert) {
        return;
    }
    ...
}

Foo a;
Foo b = DefaultFoo();
ConvertFoo(a, b);
// If conversion fails, b is unchanged

Natürlich wird dies normalerweise mit Ausnahmen behandelt. Auch wenn Ausnahmen aus irgendeinem Grund vermieden werden müssen, gibt es eine bessere Möglichkeit, dies zu tun - das TryParse-Muster ist eine Option.

Ein weiterer Grund ist, dass dies aus rein konsistenten Gründen sein könnte, zum Beispiel als Teil einer öffentlichen API, in der diese Methode für alle Konvertierungsfunktionen aus beliebigen Gründen verwendet wird (zum Beispiel für andere Konvertierungsfunktionen mit mehreren Ausgängen).

Java kann nicht gut mit mehreren Ausgaben umgehen - es kann nicht nur Ausgabeparameter wie einige Sprachen oder mehrere Rückgabewerte wie andere haben - aber Sie können auch Rückgabeobjekte verwenden.

Der Konsistenzgrund ist eher lahm, aber leider ist er der häufigste.

  • Vielleicht stammen die Stilpolizisten an Ihrem Arbeitsplatz (oder in Ihrer Codebasis) aus Ländern ohne Java und wollten sich nur ungern ändern.
  • Ihr Code ist möglicherweise ein Port aus einer Sprache, in der dieser Stil idiomatischer ist.
  • Möglicherweise muss Ihre Organisation die API-Konsistenz über verschiedene Sprachen hinweg aufrechterhalten. Dies war der Stil mit dem niedrigsten gemeinsamen Nenner (dumm, aber auch für Google ).
  • Oder vielleicht machte der Stil in der fernen Vergangenheit mehr Sinn und verwandelte sich in seine aktuelle Form (es könnte beispielsweise das TryParse-Muster gewesen sein, aber ein wohlmeinender Vorgänger hat den Rückgabewert entfernt, nachdem festgestellt wurde, dass er überhaupt nicht überprüft wurde).
congusbongus
quelle
2

Der Vorteil des zweiten Musters besteht darin, dass der Aufrufer gezwungen wird, das Eigentum und die Verantwortung für das erstellte Objekt zu übernehmen. Es steht außer Frage, ob die Methode das Objekt erstellt oder aus einem wiederverwendbaren Pool abgerufen hat. Der Anrufer weiß, dass er für die Lebensdauer und Entsorgung des neuen Objekts verantwortlich ist.

Die Nachteile dieser Methode sind:

  1. Kann nicht als Factory-Methode verwendet werden, muss der Aufrufer den genauen Untertyp kennen OtherObjectund ihn im Voraus konstruieren.
  2. Kann nicht mit Objekten verwendet werden, die Parameter in ihrem Konstruktor benötigen, wenn diese Parameter von stammen MyObject. OtherObjectmuss konstruierbar sein, ohne zu wissen MyObject.

Die Antwort basiert auf meinen Erfahrungen in C #. Ich hoffe, die Logik wird in Java übersetzt.

Rotem
quelle
Ich denke, mit der von Ihnen erwähnten Verantwortung wird auch der Lebenszyklus von Objekten "schwerer zu sehen". In einem komplexen Methodensystem würde ich es vorziehen, ein Objekt direkt zu modifizieren, anstatt alle Methoden, die wir durchlaufen, durchzugehen.
CsBalazsHungary
Das habe ich mir gedacht. Eine primitive Art von Abhängigkeitsinjektion
Lyndon White
Ein weiterer Vorteil ist, wenn OtherObject abstrakt oder eine Schnittstelle ist. Die Funktion hat keine Ahnung, welchen Typ sie erstellen soll, aber der Aufrufer könnte.
user949300
2

Angesichts der losen Semantik - myObjectToOtherObjectkönnte dies auch bedeuten, dass Sie einige Daten vom ersten zum zweiten Objekt verschieben oder dass Sie sie komplett neu konvertieren - ist die zweite Methode angemessener.

Wenn jedoch der Methodenname wäre Convert(der sollte es sein, wenn wir uns den Teil "... do the conversion" ansehen), würde ich sagen, dass die zweite Methode keinen Sinn ergibt. Sie konvertieren keinen Wert in einen anderen bereits vorhandenen Wert, IMO.

guillaume31
quelle