CODEWITHSUNDEEP
lambda
sudhagh
sudhagh

Reputation: 43

How to convert the code using Lambda java8

How can I simplify the following code using lambda java8? I am new to lambda and still learning it.

public boolean isValuePresent(String id, Optional<String> value) {
        ClassConfig config = configStorage.getConfig();
        Map<String, FeatureClass> map = config.getConfigMap();
        if (map.containsKey(id)) {
            Set<String> set = map.get(id).getset();
            if (!set.isEmpty()) {
                if (value.isPresent()) {
                    if (set.contains(value.get())) {
                        log.info(String.format("value present for id %s", id));
                        return true;
                    }
                }
            }
        }
        return false;
    }

Upvotes: 1

Views: 124

Answers (3)

Shail
Shail

Reputation: 41

You have done external looping and want to use stream API which provide us to concentrate on internal looping logic.

In stream , there are 3 parts. one is source here it is map.keySet(), second is series of intermediate operations i.e filter(), map() etc.., all operation creates new streams and does not change source, third is terminal operation i.e forEach(), collect(), anyMatch(), allMatch() etc...

Stream has consumable and lazy execution properties. That means that untill we use any termination operation, stream are not yet executed. And once termination operator is applied, stream has consumed and cannot consume further.

boolean retval = map.keySet().stream()
         .filter(x -> x.equals(id))
         .filter(x -> !map.get(x).getset().isEmpty())
         .filter(x -> value.isPresent())
         .anyMatch(x -> map.get(x).getset().contains(value.get()));

Collection API from java8 implements stream interface. Map is not part of collection API so that we are getting collection using map.keySet() and applying stream(). This became our source. filter operation takes any boolean expression here we are comparing map keys with id parameter and checking value param is present or not. anyMatch is termination operation, which will return true if any key in map will satisfy the above criteria.

Upvotes: 2

M. Prokhorov
M. Prokhorov

Reputation: 3993

In short - no. There is nothing wrong or complicated with this code that is solvable with lambdas per se. What you should rather do is review what this code actually does and simplify it from there. My take on it, is if you can not change signature of isValuePresent, then do something like this:

boolean isValuePresent(String id, Optional<String> option) {
  if (!option.isPresent()) { // this already saves us loading the config if we've got no value to check against
    return false;
  }
  FeatureClass features = configStorage.getConfig().getConfigMap().get(id);
  if (features == null) { // replaces containsKey().get() with one call. It is hardly an improvement, just my stylistic choice
    return false;
  }
  String value = option.get();
  if (features.getset().contains(value) { // we actually have no need to check if set is empty or not, we rather have it handle that itself
    log.info(String.format("value present for id %s", id));
    return true;
  }
  return false;
}

Or, if you can change signatures, I think it's better to have something like this:

boolean isValuePresent(String id, String value) {
  FeatureClass features = configStorage.getConfig().getConfigMap().get(id);
  if (features == null) {
    return false;
  }
  if (features.getset().contains(value)) {
    log.info(<snip>);
    return true;
  }
  return false;
}

And call it like this:

String id = loadId();
Optional<String> myValue = loadMyValue(id);
return myValue.map(value -> Checker.isValuePresent(id, value)).orElse(false);

As you can see, there is no lambdas, because there's no need for them - at no point here we are required to change our behavior, there's no strategy in this method that could possibly be plugged in from outside.

Now, about why I chose to not have Optional in argument:

  1. It offers more flexibility to callers of your method. If they have value String form already, they will not be required to wrap it in useless Optional before calling your method.
  2. Using Optional as method argument type looks ugly, because that class is designed for usage as return value, and doesn't offer much as argument type besides isPresent() and get(). Compare this to good number of controlled value extraction methods when it is used as return value (to name a few, orElse, orElseGet, map, flatMap, and even more to come in Java 9).
  3. java.util.Optional is not Serializable, which puts an end to call your method via remote interface (this, of course, implies that you care about calling it via remote interface).

Upvotes: 0

patwis
patwis

Reputation: 306

Does this code work for you?

for(Map.Entry<String, String> entry : map.entrySet()) {
     if(entry.getKey().equals(id)) {
         if (value.isPresent() && entry.getValue().equals(value.get())) {
             System.out.println(String.format("value present for id %s", id));
             return true;
         }
     }
}
return false;

I tried this one aswell, but return functions in lambda don't work :/

map.forEach((key, mapValue) -> {
        if(key.equals(id)) {
            if (value.isPresent() && mapValue.equals(value.get())) {
                System.out.println(String.format("value present for id %s", id));
                return true; //doesn't work -> Unexpected return value
            }
        }
    });

EDIT: I found a way to do it ^^

return map.entrySet().stream().anyMatch(entry -> entry.getKey().equals(id) && value.isPresent() && entry.getValue().equals(value.get()));

Upvotes: -1

Related Questions