15

I have below code which iterates over Cookies to reset the cookie whose name matches CookieSession.NAME

  Cookie[] cookies = httpServletRequest.getCookies();
        LOGGER.info("Clearing cookies on welcome page");
        if (cookies != null)
            for (Cookie cookie : cookies) {
                if (cookie.getName().equals(CookieSession.NAME)) {                      
                cookie.setValue(null);
                cookie.setMaxAge(0);
                cookie.setPath("/");
                httpServletResponse.addCookie(cookie);
              }
            }

can someone simplify it using java 8 lambda expression

3
  • 3
    yes. but you can start by fixing the lack of {} in your if condition
    – njzk2
    Apr 8, 2016 at 20:14
  • 7
    Not really, no. It won't be simplified; it'll be basically the same but a little more complicated. Apr 8, 2016 at 20:14
  • Lambdas aren't going to make this any simpler. Slower, more memory-heavy, but not simpler.
    – Boann
    Apr 10, 2016 at 14:34

5 Answers 5

8

Not sure if it will be simplified, but it can be done, yes:

Arrays.stream(cookies)
      .filter(c -> c.getName().equals(CookieSession.NAME))
      .forEach(cookie -> {
          cookie.setValue(null);
          cookie.setMaxAge(0);
          cookie.setPath("/");
          httpServletResponse.addCookie(cookie);
      });
2
  • 2
    Technically it won't be equivalent as OP's code is missing brackets and only the first setter will depend on the if... but anyway your code does what he expect. Apr 8, 2016 at 20:27
  • 1
    It's worth pointing out that the null check still needs to be done. httpServletRequest.getCookies() returns null if there are no cookies and Arrays.stream is not null-tolerant. Also, since HttpServletResponse is not specified as thread-safe, .forEeach better be replaced with forEachOrdered
    – Misha
    Apr 8, 2016 at 20:31
6
Arrays.stream(httpsServletRequest.getCookies())
    .filter(cookie -> CookieSession.NAME.equals(cookie.getName()))
    .forEach(cookie -> {
        cookie.setValue(null); 
        cookie.setMaxAge(0); 
        cookie.setPath("/");
        httpServletResponse.addCookie(cookie); 
    });
5

The for loop could be replaced with a forEach expression:

Arrays.stream(cookies)
      .filter(c -> c.getName().equals(CookieSession.NAME))
      .forEach(c -> {c.setValue(null);
                     c.setMaxAge(0);
                     c.setPath("/");
                     httpServletResponse.addCookie(c);
                    });
5

Other answers seem to have ignored the if (cookies != null). Also I like peek for several intermediate operations rather than a block. Seems clearer to me.

Optional.ofNullable(httpServletRequest.getCookies())
    .ifPresent(cookies -> Arrays.stream(cookies)
        .filter(cookie -> cookie.getName().equals(CookieSession.NAME))
        .peek(cookie -> cookie.setValue(null))
        .peek(cookie -> cookie.setMaxAge(0))
        .peek(cookie -> cookie.setPath("/"))
        .forEach(httpServletResponse::addCookie));
4

It is not possible to simplify this using a lambda or forEach.

Also, many people think that you should not use forEach to mutate state in a line like this:

cookies.forEach(cookie -> httpServletResponse.addCookie(cookie));

However, this is really a matter of style. As long as forEach consumes the elements sequentially (rather than in parallel), there is nothing that can actually go wrong as a result of a line like that.

Here is Oracle's advice on the subject.

In my opinion, you should leave your code as it is.

2
  • I'm concerned that your statement regarding Oracle's advice may mislead some readers. There is nothing unsafe about modifying state in a forEach. The link you provide argues against the use of forEach for stylistic reasons - as providing evidence of not thinking of a better functional solution using mapping and reduction.
    – sprinter
    Apr 9, 2016 at 21:36
  • 1
    Thanks for the edit. I'll delete my original comment to avoid confusion. I actually agree with your point about whether the new version is any better. But it seems the world is going functional so we'd better get used to it :-)
    – sprinter
    Apr 10, 2016 at 1:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.