dawidwys commented on a change in pull request #12201:
URL: https://github.com/apache/flink/pull/12201#discussion_r426407452



##########
File path: 
flink-core/src/main/java/org/apache/flink/configuration/Configuration.java
##########
@@ -909,12 +909,10 @@ private void loggingFallback(FallbackKey fallbackKey, 
ConfigOption<?> configOpti
                        List<String> listOfRawProperties = 
StructuredOptionsSplitter.splitEscaped(o.toString(), ',');
                        return listOfRawProperties.stream()
                                .map(s -> 
StructuredOptionsSplitter.splitEscaped(s, ':'))
-                               .map(pair -> {
+                               .peek(pair -> {

Review comment:
       Peek and map behave exactly the same in respect to the use cases 
described in that thread. I'd say the thread discusses the declarative aspect 
of the stream API. 
   
   In a code:
   ```
   stream()
   .map()/peek()
   .count()
   ```
   Neither `map` nor `peek` will be executed (depends on the jvm though) as 
they cannot change the result of `count()`. The linked thread rather compares 
`forEach` vs `peek`, in my opinion.
   
   In our use case particularly, the purpose of the `peek` is to add a sanity 
check right before the collection. In this case in my opinion it is absolutely 
safe to use `peek` and the code with `peek` is no different than previous 
version with `map` (minus the return value).
   
   Personally, I'd prefer to change it to `peek` as it removes IDE warning.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to