-1

The below code is intuitive in imperative style. Does trying to do the same in declarative style makes it more complex?

for (var entry : map.entrySet()) {
    if (entry.getKey().length() > 10) {
        return "invalid_key";
    } else if (entry.getValue().length() > 10) {
        return "invalid_value";
    } else if (entry.getKey().isEmpty()) {
        return "empty_key";
    } else if (entry.getValue().isEmpty()) {
        return "empty_value";
    }
}
return "all_valid";
Gopal S Akshintala
  • 1,933
  • 1
  • 17
  • 24

2 Answers2

1

You should look for the first invalid entry. If you find none, all the entries are valid. If you find an invalid entry, you check whether the key is invalid or the value is invalid.

return map.entrySet()
          .stream()
          .filter(entry -> entry.getKey().length() > 10 || entry.getValue().length() > 10)
          .findFirst()
          .map(entry -> entry.getKey().length() > 10 ? "invalid_key" : "invalid_value")
          .orElse("all_valid");
Eran
  • 387,369
  • 54
  • 702
  • 768
  • 3
    This is not at all intuitive, this defeats the purpose of readability – Gopal S Akshintala Oct 07 '19 at 10:16
  • 2
    @GopalSAkshintala in that case, why don't you keep your original code? – Eran Oct 07 '19 at 10:17
  • 1
    I guess that the readability is a subjective matter (the more you get used to Streams, the more readable it will become). We have "findFirst" in this pipeline, which is easier to understand then breaking out of a loop with a return statement to achieve the same goal. @GopalSAkshintala – Eran Oct 07 '19 at 10:22
  • @Eran The problem is not `findFirst`; it is duplicating `entry.getKey().length() > 10` and being less clear that `"invalid_value"` is returned when `entry.getValue().length() > 10` than in the original (in my opinion and I'd guess Gopal's too). Both can be avoided. – Alexey Romanov Oct 07 '19 at 12:05
  • @Eran Thanks for taking effort to answer, sorry for my rude comment, was in the developer frustration zone :) I feel this might quickly turn cumbersome, if I add more if/else conditions to check, I edited the question, can you please check. – Gopal S Akshintala Oct 08 '19 at 02:32
0

Using StreamEx, you can effectively join map and filter:

return EntryStream.of(map).mapKeyValuePartial((key, value) -> {
    if (key.length() > 10) {
        return Optional.of("invalid_key");
    } else if (value.length() > 10) {
        return Optional.of("invalid_value");
    } else {
        return Optional.empty();
    }).findFirst().orElse("all_valid");

Or with plain Stream and null:

return map.entrySet().stream().map(entry -> {
    if (entry.getKey().length() > 10) {
        return "invalid_key";
    } else if (entry.getValue().length() > 10) {
        return "invalid_value";
    } else {
        return null;
    }).filter(x -> x != null).findFirst().orElse("all_valid");
Alexey Romanov
  • 167,066
  • 35
  • 309
  • 487