1

I have read this post How to use if-else logic in Java 8 stream forEach

The whole point of Java 8 is to write more elegant, readable and concise code. I rewrote this execute method utilizing Java 8 Streams and Lambda but what i end up with doesn't look any different from the Java 7 version.

private static boolean execute(String expression) {
    Deque<Character> stack = new ArrayDeque<>();
    for (char c : expression.toCharArray()) {
        if (isOpenToken(c)) {
            stack.push(c);
        } else if (stack.isEmpty() || !matches(stack.pop(), c)) {
            return false;
        }
    }
    return stack.isEmpty();
}

This is what the method looks like now in Java 8

private static boolean execute(String expression) {
    char[] a = expression.toCharArray();
    Deque<Character> stack = new ArrayDeque<>();
    IntStream.range(0, a.length).forEach(i -> {
        if (isOpenToken(a[i])) {
            stack.push(a[i]);
        } else {
            matches(stack.pop(), a[i]);
        }
    });
    return stack.isEmpty();
}

Is there a more elegant way of doing this in Java 8?

Andronicus
  • 25,419
  • 17
  • 47
  • 88
theyCallMeJun
  • 911
  • 1
  • 10
  • 21
  • 8
    It is a common misconception that everything will "become more elegant" when you use streams. This code right now is perfectly fine, you should not replace _every_ for loop in your code to use streams. – Sweeper Mar 09 '19 at 10:30
  • You might be interested in [chars()](https://docs.oracle.com/javase/8/docs/api/java/lang/CharSequence.html#chars--) method from String. – Fureeish Mar 09 '19 at 10:32
  • You should as well note that your stream version and your initial Java 7 solution won’t do the same. In the first version you have an early exit if `!matches(stack.pop(), c)`. This is gone in the stream version. – dpr Mar 09 '19 at 20:54
  • @dpr Yes both versions produce the same result but go about it differently. I reckon though that the Java 8 would be slower because it has to go through all items in the list. This is because you can't have `break` or `return` using `forEach`. However, in the end `stack.isEmpty()` can either be true or false. I have decided to use the Java 7 version. – theyCallMeJun Mar 10 '19 at 00:23
  • Upvote for not converting to streams. – dpr Mar 10 '19 at 07:30
  • Is `matches` doing anything apart from performing some checks? – dpr Mar 11 '19 at 08:19
  • Additionally to the early exit, the stream version will fail, if the first character of the expression `a[0]` fails the `isOpenToken` check. With the Java7 version nothing will be done as the stack is empty. The stream version will throw an `EmptyStackException` as calling `stack.pop()` on an empty stack is not allowed. – dpr Mar 11 '19 at 08:32
  • You could use `return expression.chars().allMatch(c -> isOpenToken(c) && stack.add(c) || matches(stack.removeLast(), c));` if you change the uses of `char`/`Character` to `int`/`Integer`, however, that’s not the intended usage of the Stream API. – Holger Mar 11 '19 at 12:29

1 Answers1

1

Not much, that can be done here, streams are great for mapping, filtering, aggregating, collecting... Here you only have side effects for every element. You might wanna use some of functional goodness of streams by applying mapping:

IntStream.range(0, a.length).map(i -> a[i]).forEach(el -> {
    if (isOpenToken(el)) {
        stack.push(el);
    } else {
        matches(stack.pop(), el);
    }
});
Andronicus
  • 25,419
  • 17
  • 47
  • 88
  • It would make more sense to use `expression.chars()` to get an `IntStream` over the chars without copying everything into an array first. – Holger Mar 11 '19 at 12:03