Rule 3864 "Stream.peek should not be used": improving title and description

Clean Code New rules / language support
1

Hi all,

the recently introduced rule about Stream.peek() works fine, but its current title and description lack detail:

“Stream.peek” should not be used

Labels: none

According to the JavaDocs java.util.Stream.peek is only meant to be used for debugging purposes and should not be used in production.

Noncompliant Code Example

Stream.of("one", "two", "three", "four")
         .filter(e -> e.length() > 3)
         .peek(e -> System.out.println("Filtered value: " + e)); // Noncompliant

See

Java 8 API Documentation

Compared to other rules, this description does not give much rationale, background or advice. Also, its title is slightly misleading: “debugging purposes” does not really imply “should not be used in production” as production code certainly can have logging calls which are useful for debugging.

I suggest the following title, labels and description instead:

“Stream.peek” should be used with caution

Labels: +pitfall, +java8

According to its JavaDocs, java.util.Stream.peek() “exists mainly to support debugging” purposes. While non-debugging usages are neither forbidden nor discouraged, relying on peek() without careful consideration can lead to error-prone code due to its subtleties:

  • If the stream pipeline neither includes a terminal operation nor a stateful intermediate operation, no elements may be consumed at all, so the action will not be invoked.
  • In cases where the stream implementation is able to optimize away the production of some or all the elements (such as with short-circuiting operations like findFirst, or in certain situations with count), the action will not be invoked for those elements.

This rule raises an issue for each use of peek() to be sure that it is challenged and validated by the team to be meant for production debugging/logging purposes and/or is not affected by the subtleties given above.

Noncompliant Code Example

Stream.of("one", "two", "three", "four")
         .filter(e -> e.length() > 3)
         .peek(e -> System.out.println("Filtered value: " + e)); // Noncompliant

See

Notes

Please consider using the text above or parts of it. If nothing else, I suggest adding the Idiomatic Peeking with Java Stream API link to the existing rule description.

Best regards,
Jens

2

Hello,

I took some of your suggestions.

This is coming from the JavaDocs itself, so I can’t take as is. I need at minimum to quote the original author.

Can you tell me if you took this from a web site or this is your own sentence ?

If the stream pipeline neither includes a terminal operation nor a stateful intermediate operation, no elements may be consumed at all, so the action will not be invoked

Thanks

3

Ah, I wasn’t thinking of copyrights. Makes sense.

If the stream pipeline neither includes a terminal operation nor a stateful intermediate operation, no elements may be consumed at all, so the action will not be invoked

This is my own sentence. I revisited my sources to rule out a case of subconscious copying. However, while doing so I noticed that my sentence could be improved: it may lead the reader to believe that having a stateful intermediate operation guarantees that some/all elements will be consumed, which is false if you have no terminal operation. So let’s go with this instead (still my own words):

If the stream pipeline does not include a terminal operation, no elements will be consumed and the peek() action will not be invoked at all.

I couldn’t come up with a way to include the aspect of stateful intermediate operations correctly, but I guess it will confuse the reader more than it helps.

Also, the “optimize away” bit is a important, so you should either keep the JavaDoc quote or go with something like this (my own sentence):

As long as a stream implementation can reach the final goal, it can freely optimize processing by only producing some elements or even none at all (e.g. relying on other collection methods for counting elements). Accordingly, the peek() action will be invoked for fewer elements or not at all.

Best regards,
Jens

4

Also, this part is taken verbatim from the “idiomatic peeking” article, but it’s not really worth keeping and making it a formal quote. Maybe go for something like this:

Although this does not mean that using it for other purposes is discouraged, …

5

https://jira.sonarsource.com/browse/RSPEC-3864 has been fixed with all your feedback.

Thanks!

1 Like