How to avoid nested forEach calls?











up vote
6
down vote

favorite
1












I have the following code:



interface Device {
// ...
boolean isDisconnected();
void reconnect();
}

interface Gateway {
// ...
List<Device> getDevices();
}

...

for (Gateway gateway : gateways) {
for(Device device : gateway.getDevices()){
if(device.isDisconnected()){
device.reconnect();
}
}
}


I want to refactor the code using Stream API. My first attempt was like the following:



gateways
.stream()
.forEach(
gateway -> {
gateway
.getDevices()
.parallelStream()
.filter(device -> device.isDisconnected())
.forEach(device -> device.reconnect())
;
}
)
;


I didn't like it so after some modifications I ended up with this code:



gateways
.parallelStream()
.map(gateway -> gateway.getDevices().parallelStream())
.map(stream -> stream.filter(device -> device.isDisconnected()))
.forEach(stream -> stream.forEach(device -> device.reconnect()))
;


My question is whether there is a way to avoid nested forEach.










share|improve this question




























    up vote
    6
    down vote

    favorite
    1












    I have the following code:



    interface Device {
    // ...
    boolean isDisconnected();
    void reconnect();
    }

    interface Gateway {
    // ...
    List<Device> getDevices();
    }

    ...

    for (Gateway gateway : gateways) {
    for(Device device : gateway.getDevices()){
    if(device.isDisconnected()){
    device.reconnect();
    }
    }
    }


    I want to refactor the code using Stream API. My first attempt was like the following:



    gateways
    .stream()
    .forEach(
    gateway -> {
    gateway
    .getDevices()
    .parallelStream()
    .filter(device -> device.isDisconnected())
    .forEach(device -> device.reconnect())
    ;
    }
    )
    ;


    I didn't like it so after some modifications I ended up with this code:



    gateways
    .parallelStream()
    .map(gateway -> gateway.getDevices().parallelStream())
    .map(stream -> stream.filter(device -> device.isDisconnected()))
    .forEach(stream -> stream.forEach(device -> device.reconnect()))
    ;


    My question is whether there is a way to avoid nested forEach.










    share|improve this question


























      up vote
      6
      down vote

      favorite
      1









      up vote
      6
      down vote

      favorite
      1






      1





      I have the following code:



      interface Device {
      // ...
      boolean isDisconnected();
      void reconnect();
      }

      interface Gateway {
      // ...
      List<Device> getDevices();
      }

      ...

      for (Gateway gateway : gateways) {
      for(Device device : gateway.getDevices()){
      if(device.isDisconnected()){
      device.reconnect();
      }
      }
      }


      I want to refactor the code using Stream API. My first attempt was like the following:



      gateways
      .stream()
      .forEach(
      gateway -> {
      gateway
      .getDevices()
      .parallelStream()
      .filter(device -> device.isDisconnected())
      .forEach(device -> device.reconnect())
      ;
      }
      )
      ;


      I didn't like it so after some modifications I ended up with this code:



      gateways
      .parallelStream()
      .map(gateway -> gateway.getDevices().parallelStream())
      .map(stream -> stream.filter(device -> device.isDisconnected()))
      .forEach(stream -> stream.forEach(device -> device.reconnect()))
      ;


      My question is whether there is a way to avoid nested forEach.










      share|improve this question















      I have the following code:



      interface Device {
      // ...
      boolean isDisconnected();
      void reconnect();
      }

      interface Gateway {
      // ...
      List<Device> getDevices();
      }

      ...

      for (Gateway gateway : gateways) {
      for(Device device : gateway.getDevices()){
      if(device.isDisconnected()){
      device.reconnect();
      }
      }
      }


      I want to refactor the code using Stream API. My first attempt was like the following:



      gateways
      .stream()
      .forEach(
      gateway -> {
      gateway
      .getDevices()
      .parallelStream()
      .filter(device -> device.isDisconnected())
      .forEach(device -> device.reconnect())
      ;
      }
      )
      ;


      I didn't like it so after some modifications I ended up with this code:



      gateways
      .parallelStream()
      .map(gateway -> gateway.getDevices().parallelStream())
      .map(stream -> stream.filter(device -> device.isDisconnected()))
      .forEach(stream -> stream.forEach(device -> device.reconnect()))
      ;


      My question is whether there is a way to avoid nested forEach.







      java collections foreach java-8 java-stream






      share|improve this question















      share|improve this question













      share|improve this question




      share|improve this question








      edited 1 hour ago









      candied_orange

      4,2371647




      4,2371647










      asked 8 hours ago









      Hans van Kessels

      659




      659
























          3 Answers
          3






          active

          oldest

          votes

















          up vote
          5
          down vote



          accepted










          You should flatten the stream of streams using flatMap instead of map:



          gateways
          .parallelStream()
          .flatMap(gateway -> gateway.getDevices().parallelStream())
          .filter(device -> device.isDisconnected())
          .forEach(device -> device.reconnect())
          ;




          I would improve it further by using method references instead of lambda expressions:



          gateways
          .parallelStream()
          .map(Gateway::getDevices)
          .flatMap(List::stream)
          .filter(Device::isDisconnected)
          .forEach(Device::reconnect)
          ;





          share|improve this answer























          • Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
            – Hans van Kessels
            8 hours ago






          • 1




            Yes, it does. Please read this post for more detailed explanation.
            – ETO
            8 hours ago


















          up vote
          6
          down vote













          Don't refactor your code into using Streams. You gain no benefits and gain no advantages over doing it like this, since the code is now less readable and less idiomatic for future maintainers.



          By not using streams, you avoid nested forEach statements.



          Remember: streams are meant to be side-effect free for safer parallelization. forEach by definition introduces side-effects. You lose the benefit of streams and lose readability at the same time, making this less desirable to do at all.






          share|improve this answer























          • Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
            – Hans van Kessels
            8 hours ago








          • 1




            One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
            – Makoto
            8 hours ago


















          up vote
          2
          down vote













          I would try this with a sequential stream before using a parallel one:



          gateways
          .stream()
          .flatMap(gateway -> gateway.getDevices().stream())
          .filter(device -> device.isDisconnected())
          .forEach(device -> device.reconnect())
          ;


          The idea is to create a stream via gateways.stream() then flatten the sequences returned from gateway.getDevices() via flatMap.



          Then we apply a filter operation which acts like the if statement in your code and finally, a forEach terminal operation enabling us to invoke reconnect on each and every device passing the filter operation.



          see Should I always use a parallel stream when possible?






          share|improve this answer























            Your Answer






            StackExchange.ifUsing("editor", function () {
            StackExchange.using("externalEditor", function () {
            StackExchange.using("snippets", function () {
            StackExchange.snippets.init();
            });
            });
            }, "code-snippets");

            StackExchange.ready(function() {
            var channelOptions = {
            tags: "".split(" "),
            id: "1"
            };
            initTagRenderer("".split(" "), "".split(" "), channelOptions);

            StackExchange.using("externalEditor", function() {
            // Have to fire editor after snippets, if snippets enabled
            if (StackExchange.settings.snippets.snippetsEnabled) {
            StackExchange.using("snippets", function() {
            createEditor();
            });
            }
            else {
            createEditor();
            }
            });

            function createEditor() {
            StackExchange.prepareEditor({
            heartbeatType: 'answer',
            convertImagesToLinks: true,
            noModals: true,
            showLowRepImageUploadWarning: true,
            reputationToPostImages: 10,
            bindNavPrevention: true,
            postfix: "",
            imageUploader: {
            brandingHtml: "Powered by u003ca class="icon-imgur-white" href="https://imgur.com/"u003eu003c/au003e",
            contentPolicyHtml: "User contributions licensed under u003ca href="https://creativecommons.org/licenses/by-sa/3.0/"u003ecc by-sa 3.0 with attribution requiredu003c/au003e u003ca href="https://stackoverflow.com/legal/content-policy"u003e(content policy)u003c/au003e",
            allowUrls: true
            },
            onDemand: true,
            discardSelector: ".discard-answer"
            ,immediatelyShowMarkdownHelp:true
            });


            }
            });














            draft saved

            draft discarded


















            StackExchange.ready(
            function () {
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53823642%2fhow-to-avoid-nested-foreach-calls%23new-answer', 'question_page');
            }
            );

            Post as a guest















            Required, but never shown

























            3 Answers
            3






            active

            oldest

            votes








            3 Answers
            3






            active

            oldest

            votes









            active

            oldest

            votes






            active

            oldest

            votes








            up vote
            5
            down vote



            accepted










            You should flatten the stream of streams using flatMap instead of map:



            gateways
            .parallelStream()
            .flatMap(gateway -> gateway.getDevices().parallelStream())
            .filter(device -> device.isDisconnected())
            .forEach(device -> device.reconnect())
            ;




            I would improve it further by using method references instead of lambda expressions:



            gateways
            .parallelStream()
            .map(Gateway::getDevices)
            .flatMap(List::stream)
            .filter(Device::isDisconnected)
            .forEach(Device::reconnect)
            ;





            share|improve this answer























            • Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
              – Hans van Kessels
              8 hours ago






            • 1




              Yes, it does. Please read this post for more detailed explanation.
              – ETO
              8 hours ago















            up vote
            5
            down vote



            accepted










            You should flatten the stream of streams using flatMap instead of map:



            gateways
            .parallelStream()
            .flatMap(gateway -> gateway.getDevices().parallelStream())
            .filter(device -> device.isDisconnected())
            .forEach(device -> device.reconnect())
            ;




            I would improve it further by using method references instead of lambda expressions:



            gateways
            .parallelStream()
            .map(Gateway::getDevices)
            .flatMap(List::stream)
            .filter(Device::isDisconnected)
            .forEach(Device::reconnect)
            ;





            share|improve this answer























            • Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
              – Hans van Kessels
              8 hours ago






            • 1




              Yes, it does. Please read this post for more detailed explanation.
              – ETO
              8 hours ago













            up vote
            5
            down vote



            accepted







            up vote
            5
            down vote



            accepted






            You should flatten the stream of streams using flatMap instead of map:



            gateways
            .parallelStream()
            .flatMap(gateway -> gateway.getDevices().parallelStream())
            .filter(device -> device.isDisconnected())
            .forEach(device -> device.reconnect())
            ;




            I would improve it further by using method references instead of lambda expressions:



            gateways
            .parallelStream()
            .map(Gateway::getDevices)
            .flatMap(List::stream)
            .filter(Device::isDisconnected)
            .forEach(Device::reconnect)
            ;





            share|improve this answer














            You should flatten the stream of streams using flatMap instead of map:



            gateways
            .parallelStream()
            .flatMap(gateway -> gateway.getDevices().parallelStream())
            .filter(device -> device.isDisconnected())
            .forEach(device -> device.reconnect())
            ;




            I would improve it further by using method references instead of lambda expressions:



            gateways
            .parallelStream()
            .map(Gateway::getDevices)
            .flatMap(List::stream)
            .filter(Device::isDisconnected)
            .forEach(Device::reconnect)
            ;






            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited 1 hour ago









            candied_orange

            4,2371647




            4,2371647










            answered 8 hours ago









            ETO

            1,416218




            1,416218












            • Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
              – Hans van Kessels
              8 hours ago






            • 1




              Yes, it does. Please read this post for more detailed explanation.
              – ETO
              8 hours ago


















            • Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
              – Hans van Kessels
              8 hours ago






            • 1




              Yes, it does. Please read this post for more detailed explanation.
              – ETO
              8 hours ago
















            Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
            – Hans van Kessels
            8 hours ago




            Thank you, it works! Does the code with method references have the same performance as the first one? It looks pretty elegant.
            – Hans van Kessels
            8 hours ago




            1




            1




            Yes, it does. Please read this post for more detailed explanation.
            – ETO
            8 hours ago




            Yes, it does. Please read this post for more detailed explanation.
            – ETO
            8 hours ago












            up vote
            6
            down vote













            Don't refactor your code into using Streams. You gain no benefits and gain no advantages over doing it like this, since the code is now less readable and less idiomatic for future maintainers.



            By not using streams, you avoid nested forEach statements.



            Remember: streams are meant to be side-effect free for safer parallelization. forEach by definition introduces side-effects. You lose the benefit of streams and lose readability at the same time, making this less desirable to do at all.






            share|improve this answer























            • Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
              – Hans van Kessels
              8 hours ago








            • 1




              One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
              – Makoto
              8 hours ago















            up vote
            6
            down vote













            Don't refactor your code into using Streams. You gain no benefits and gain no advantages over doing it like this, since the code is now less readable and less idiomatic for future maintainers.



            By not using streams, you avoid nested forEach statements.



            Remember: streams are meant to be side-effect free for safer parallelization. forEach by definition introduces side-effects. You lose the benefit of streams and lose readability at the same time, making this less desirable to do at all.






            share|improve this answer























            • Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
              – Hans van Kessels
              8 hours ago








            • 1




              One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
              – Makoto
              8 hours ago













            up vote
            6
            down vote










            up vote
            6
            down vote









            Don't refactor your code into using Streams. You gain no benefits and gain no advantages over doing it like this, since the code is now less readable and less idiomatic for future maintainers.



            By not using streams, you avoid nested forEach statements.



            Remember: streams are meant to be side-effect free for safer parallelization. forEach by definition introduces side-effects. You lose the benefit of streams and lose readability at the same time, making this less desirable to do at all.






            share|improve this answer














            Don't refactor your code into using Streams. You gain no benefits and gain no advantages over doing it like this, since the code is now less readable and less idiomatic for future maintainers.



            By not using streams, you avoid nested forEach statements.



            Remember: streams are meant to be side-effect free for safer parallelization. forEach by definition introduces side-effects. You lose the benefit of streams and lose readability at the same time, making this less desirable to do at all.







            share|improve this answer














            share|improve this answer



            share|improve this answer








            edited 8 hours ago

























            answered 8 hours ago









            Makoto

            80.2k15125170




            80.2k15125170












            • Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
              – Hans van Kessels
              8 hours ago








            • 1




              One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
              – Makoto
              8 hours ago


















            • Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
              – Hans van Kessels
              8 hours ago








            • 1




              One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
              – Makoto
              8 hours ago
















            Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
            – Hans van Kessels
            8 hours ago






            Thanks for your answer. This is just my in-home project. I'm trying to learn Stream API by doing some pseudo-real tasks.
            – Hans van Kessels
            8 hours ago






            1




            1




            One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
            – Makoto
            8 hours ago




            One way to learn about the Stream API is to know when not to apply it. This feels like one of those times, especially since you're not guaranteed that the operations you're performing inside of the stream are side-effect free which would make parallelization safe.
            – Makoto
            8 hours ago










            up vote
            2
            down vote













            I would try this with a sequential stream before using a parallel one:



            gateways
            .stream()
            .flatMap(gateway -> gateway.getDevices().stream())
            .filter(device -> device.isDisconnected())
            .forEach(device -> device.reconnect())
            ;


            The idea is to create a stream via gateways.stream() then flatten the sequences returned from gateway.getDevices() via flatMap.



            Then we apply a filter operation which acts like the if statement in your code and finally, a forEach terminal operation enabling us to invoke reconnect on each and every device passing the filter operation.



            see Should I always use a parallel stream when possible?






            share|improve this answer



























              up vote
              2
              down vote













              I would try this with a sequential stream before using a parallel one:



              gateways
              .stream()
              .flatMap(gateway -> gateway.getDevices().stream())
              .filter(device -> device.isDisconnected())
              .forEach(device -> device.reconnect())
              ;


              The idea is to create a stream via gateways.stream() then flatten the sequences returned from gateway.getDevices() via flatMap.



              Then we apply a filter operation which acts like the if statement in your code and finally, a forEach terminal operation enabling us to invoke reconnect on each and every device passing the filter operation.



              see Should I always use a parallel stream when possible?






              share|improve this answer

























                up vote
                2
                down vote










                up vote
                2
                down vote









                I would try this with a sequential stream before using a parallel one:



                gateways
                .stream()
                .flatMap(gateway -> gateway.getDevices().stream())
                .filter(device -> device.isDisconnected())
                .forEach(device -> device.reconnect())
                ;


                The idea is to create a stream via gateways.stream() then flatten the sequences returned from gateway.getDevices() via flatMap.



                Then we apply a filter operation which acts like the if statement in your code and finally, a forEach terminal operation enabling us to invoke reconnect on each and every device passing the filter operation.



                see Should I always use a parallel stream when possible?






                share|improve this answer














                I would try this with a sequential stream before using a parallel one:



                gateways
                .stream()
                .flatMap(gateway -> gateway.getDevices().stream())
                .filter(device -> device.isDisconnected())
                .forEach(device -> device.reconnect())
                ;


                The idea is to create a stream via gateways.stream() then flatten the sequences returned from gateway.getDevices() via flatMap.



                Then we apply a filter operation which acts like the if statement in your code and finally, a forEach terminal operation enabling us to invoke reconnect on each and every device passing the filter operation.



                see Should I always use a parallel stream when possible?







                share|improve this answer














                share|improve this answer



                share|improve this answer








                edited 1 hour ago









                candied_orange

                4,2371647




                4,2371647










                answered 8 hours ago









                Aomine

                36.6k72961




                36.6k72961






























                    draft saved

                    draft discarded




















































                    Thanks for contributing an answer to Stack Overflow!


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    To learn more, see our tips on writing great answers.





                    Some of your past answers have not been well-received, and you're in danger of being blocked from answering.


                    Please pay close attention to the following guidance:


                    • Please be sure to answer the question. Provide details and share your research!

                    But avoid



                    • Asking for help, clarification, or responding to other answers.

                    • Making statements based on opinion; back them up with references or personal experience.


                    To learn more, see our tips on writing great answers.




                    draft saved


                    draft discarded














                    StackExchange.ready(
                    function () {
                    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fstackoverflow.com%2fquestions%2f53823642%2fhow-to-avoid-nested-foreach-calls%23new-answer', 'question_page');
                    }
                    );

                    Post as a guest















                    Required, but never shown





















































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown

































                    Required, but never shown














                    Required, but never shown












                    Required, but never shown







                    Required, but never shown







                    Popular posts from this blog

                    404 Error Contact Form 7 ajax form submitting

                    How to know if a Active Directory user can login interactively

                    TypeError: fit_transform() missing 1 required positional argument: 'X'