Ist es ein Antipattern, ein Stream-Element mit peek () zu ändern?

36

Angenommen, ich habe einen Datenstrom von Dingen und möchte diese in der Mitte des Datenstroms "anreichern". peek()Dazu kann ich Folgendes verwenden :

streamOfThings.peek(this::thingMutator).forEach(this::someConsumer);

Nehmen Sie an, dass das Mutieren der Dinge an dieser Stelle im Code korrekt ist - zum Beispiel das thingMutator Methode das Feld "lastProcessed" möglicherweise auf die aktuelle Zeit.

Jedoch, peek() meisten Kontexten bedeutet es jedoch "schauen, aber nicht anfassen".

Ist die Verwendung peek()zum Mutieren von Stream-Elementen ein Gegenmuster oder eine schlechte Empfehlung?

Bearbeiten:

Der alternative, konventionellere Ansatz wäre die Umstellung des Verbrauchers:

private void thingMutator(Thing thing) {
    thing.setLastProcessed(System.currentTimeMillis());
}

zu einer Funktion, die den Parameter zurückgibt:

private Thing thingMutator(Thing thing) {
    thing.setLastProcessed(currentTimeMillis());
    return thing;
}

und benutze map()stattdessen:

stream.map(this::thingMutator)...

Aber das einleitet perfunctory Code ( return) und ich bin nicht davon überzeugt , es ist klarer, weil Sie wissen , peek()kehren das gleiche Objekt, aber mit map()ihm nicht einmal auf einem Blick klar ist , dass es die gleiche Klasse des Objekts.

Weiter kann mit peek()dir ein Lambda sein, das mutiert, aber mit map()dir musst du ein Zugunglück bauen. Vergleichen Sie:

stream.peek(t -> t.setLastProcessed(currentTimeMillis())).forEach(...)
stream.map(t -> {t.setLastProcessed(currentTimeMillis()); return t;}).forEach(...)

Ich denke der peek() Version ist klarer und das Lambda mutiert eindeutig, so dass es keine "mysteriösen" Nebenwirkungen gibt. In ähnlicher Weise ist auch dies klar und offensichtlich, wenn eine Methodenreferenz verwendet wird und der Name der Methode eindeutig eine Mutation impliziert.

Persönlich scheue ich mich nicht davor, peek()zu mutieren - ich finde es sehr praktisch.

Bohemien
quelle
1
Haben Sie peekmit einem Stream gearbeitet, der seine Elemente dynamisch generiert? Funktioniert es noch oder gehen die Änderungen verloren? Das Ändern der Elemente eines Streams klingt für mich unzuverlässig.
Sebastian Redl
@SebastianRedl Ich habe es 100% zuverlässig gefunden. Ich habe es zum ersten Mal verwendet, um während der Verarbeitung zu sammeln: List<Thing> list; things.stream().peek(list::add).forEach(...);sehr praktisch. In letzter Zeit. Ich habe es verwendet Infos hinzufügen für die Veröffentlichung: Map<Thing, Long> timestamps = ...; return things.stream().peek(t -> t.setTimestamp(timestamp.get(t))).collect(toList());. Ich weiß, dass es andere Möglichkeiten gibt, dieses Beispiel umzusetzen, aber ich vereinfache es hier stark. Mit peek()kompakter und eleganter Code IMHO. Abgesehen von der Lesbarkeit handelt es sich bei dieser Frage wirklich um das, was Sie angesprochen haben. Ist es sicher / zuverlässig?
Bohemian
2
Die Frage In Java-Streams ist Peek eigentlich nur zum Debuggen? kann auch interessant sein.
Vincent Savard
@Bohemien. Üben Sie diesen Ansatz noch mit peek? Ich habe eine ähnliche Frage zum Stackoverflow und hoffe, Sie könnten es sich ansehen und Ihr Feedback geben. Vielen Dank. stackoverflow.com/questions/47356992/…
tsolakp

Antworten:

23

Sie haben Recht, "spähen" im englischen Sinne bedeutet "gucken, aber nicht anfassen".

Doch die JavaDoc heißt es :

spähen

Stream Peek (Verbraucheraktion)

Gibt einen Stream zurück, der aus den Elementen dieses Streams besteht, und führt zusätzlich die bereitgestellte Aktion für jedes Element aus, wenn Elemente aus dem resultierenden Stream verbraucht werden.

Schlüsselwörter: "Performing ... Action" und "Consumed". Das JavaDoc ist sehr klar, dass wir erwarten sollten peek, die Möglichkeit zu haben, den Stream zu modifizieren.

In JavaDoc heißt es jedoch auch:

API-Hinweis:

Diese Methode dient hauptsächlich zur Unterstützung des Debuggens, bei dem Sie die Elemente sehen möchten, wenn sie an einem bestimmten Punkt in einer Pipeline vorbeifließen

Dies weist darauf hin, dass es eher zur Beobachtung gedacht ist, z. B. zum Protokollieren von Elementen im Stream.

Ich gehe davon aus, dass wir Aktionen mit den Elementen im Stream ausführen können , aber vermeiden sollten, Elemente im Stream zu mutieren . Rufen Sie beispielsweise Methoden für die Objekte auf, vermeiden Sie jedoch Mutationen.

Zumindest möchte ich Ihrem Code einen kurzen Kommentar hinzufügen:

// Note: this peek() call will modify the Things in the stream.
streamOfThings.peek(this::thingMutator).forEach(this::someConsumer);

In Bezug auf die Nützlichkeit solcher Kommentare gehen die Meinungen auseinander, aber ich würde in diesem Fall einen solchen Kommentar verwenden.


quelle
1
Warum benötigen Sie einen Kommentar, wenn der Methodenname eindeutig auf eine Mutation hinweist , z. B. thingMutatoroder konkreter resetLastProcessedusw. Sofern kein zwingender Grund vorliegt, weisen die erforderlichen Kommentare wie Ihr Vorschlag in der Regel auf eine schlechte Auswahl von Variablen- und / oder Methodennamen hin. Wenn gute Namen gewählt werden, ist das nicht genug? Oder sagen Sie, dass trotz eines guten Namens alles in peek()wie ein blinder Fleck ist, den die meisten Programmierer "überfliegen" (visuell überspringen) würden? Außerdem ist "hauptsächlich zum Debuggen" nicht dasselbe wie "nur zum Debuggen" - welche anderen Verwendungen als "hauptsächlich" waren vorgesehen?
Bohemian
@ Bohemian Ich würde es hauptsächlich erklären, weil der Methodenname nicht intuitiv ist. Und ich arbeite nicht für Oracle. Ich bin nicht in ihrem Java-Team. Ich habe keine Ahnung, was sie vorhatten.
@ Bohemian, denn wenn ich suche, was die Elemente in einer Weise verändert, die ich nicht mag, höre ich auf, die Zeile bei "peek" zu lesen, weil "peek" nichts ändert. Erst später, wenn alle anderen Code-Stellen nicht den Fehler enthielten, den ich verfolge, werde ich darauf zurückkommen, möglicherweise mit einem Debugger bewaffnet, und dann vielleicht "omg, wer hat diesen irreführenden Code dort abgelegt ?!"
Frank Hopkins
@Bohemian in der Exampe hier, wo sich alles in einer Zeile befindet, muss man sowieso weiterlesen, aber man kann den Inhalt von "peek" überspringen und oft geht eine Stream-Anweisung mit einem Stream-Vorgang pro Zeile über mehrere Zeilen
Frank Hopkins
1

Es könnte leicht falsch interpretiert werden, daher würde ich es vermeiden, es so zu verwenden. Die wahrscheinlich beste Option ist die Verwendung einer Lambda-Funktion zum Zusammenführen der beiden erforderlichen Aktionen in den forEach-Aufruf. Möglicherweise möchten Sie auch ein neues Objekt zurückgeben, anstatt das vorhandene zu mutieren. Es ist möglicherweise etwas weniger effizient, führt jedoch wahrscheinlich zu besser lesbarem Code und verringert das Risiko, dass die geänderte Liste versehentlich für einen anderen Vorgang verwendet wird, der ausgeführt werden sollte habe das Original erhalten.

Jules
quelle
-2

Der API-Hinweis sagt uns, dass die Methode hauptsächlich für Aktionen wie Debuggen / Protokollieren / Auslösen von Statistiken usw. hinzugefügt wurde.

@apiNote Diese Methode dient hauptsächlich zur Unterstützung des Debuggens, bei dem * die Elemente angezeigt werden sollen, wenn sie an einem bestimmten Punkt in einer Pipeline vorbeifließen: *

       {@Code
     * Stream.of ("eins", "zwei", "drei", "vier")
     * .filter (e -> e.length ()> 3)
     * .peek (e -> System.out.println ("Gefilterter Wert:" + e))
     * .map (String :: toUpperCase)
     * .peek (e -> System.out.println ("Zugeordneter Wert:" + e))
     * .collect (Collectors.toList ());
     *}

javaProgrammer
quelle
2
Ihre Antwort wird bereits von Snowman's abgedeckt.
Vincent Savard
3
Und "hauptsächlich" bedeutet nicht "nur".
Bohemian