16

Let consider a below HashMap

HashMap<String, String> map = new HashMap<String, String>();

I have values in map like

map.put("model", "test");

Currently, if I want to get value from map I am doing

if(map!=null){
 if(map.get("model")!=null && !map.get("model").isEmpty()){
   //some logic
 }
}

Is there any better approach in Java 8 by using Optional or Lambdas to achieve the above condition?

Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
ppb
  • 2,299
  • 4
  • 43
  • 75

5 Answers5

13

First of all, your Map should not be null. Never. It could be empty, but there's no reason for it to be null. So that eliminates the first null check.

Now, unfortunately, Java doesn't have such a utility method, but several commonly used libs (apache commons, Guava, etc.) have it, or you can write it yourself, so it becomes:

String model = map.get("model");
if (!Strings.isEmptyOrNull(model)) {
    // do somthing
}

Using Optional to wrap a nullable value as part of your logic is considered an antipattern. Optional is designed to be used as a return type. So I wouldn't advide using it here.

Also note that it feels like you're using a map to store attributes of an object If it is so, then consider defining a real class, with typed properties, instead of using a map.

JB Nizet
  • 678,734
  • 91
  • 1,224
  • 1,255
  • Can you provide a link for further discussion on the topic of `Optional` being an antipattern in this case? I also read it before and think a link would be helpful for readers. – Zabuzard Nov 08 '17 at 23:07
  • 1
    https://www.youtube.com/watch?v=Ej0sss6cq14, https://stackoverflow.com/a/23464794/571407, – JB Nizet Nov 08 '17 at 23:11
  • Re the Map not being null, you don't know that. The line showing map being instantiated could be a field, which is not readonly, and it could be getting changed elsewhere. – LordWilmore Dec 06 '17 at 11:58
  • 2
    @LordWilmore my point is that is **should** never be null. It would be a design mistake to have a null HashMap. So, if your design is clean, the HashMap should never be null, and you should thus not care about that: if the HashMap is null, it's a bug, and throwing a NullPointerException is the right thing to do. – JB Nizet Dec 06 '17 at 16:09
  • @JBNizet quite agree, this was just an observation, but this place is full of people asking about bad code – LordWilmore Dec 06 '17 at 16:30
  • agreed with the Map should not be null. – Chathura Buddhika Apr 06 '21 at 06:58
10

Not sure why you check if the map is null after just having created it, but here goes:

Optional.ofNullable(map)
    .map(m -> m.getOrDefault("model", "")) // Use an empty String if not present
    .filter(s -> !s.isEmpty())             // Filter all empty values
    .ifPresent(valueString -> {            // Check if value is present
        // Logic here
});

Or in one line:

Optional.ofNullable(map).map(m -> m.getOrDefault("model", "")).filter(s -> !s.isEmpty()).ifPresent(valueString -> {
        // Logic here
});

Change ifPresent to map if you want to return something; i.e. Optional of whatever you calculate.

smac89
  • 39,374
  • 15
  • 132
  • 179
  • I am converting json request body in to map, sometime there is a chance to absent of some inner json object, thats why I am checking null for map here. – ppb Nov 08 '17 at 23:15
  • @ppb Check out jackson https://github.com/FasterXML/jackson to handle conversion of json to java objects. – Bob Brinks Nov 09 '17 at 10:28
4

If you are interested in an Optional approach,

You can wrap a map.get("model") value into an Optional.ofNullable and do filter work by the Predicate<String> value -> !value.isEmpty():

if (isNull(map)) { // import static java.util.Objects.isNull;
    return;        // to minimise nesting
}

Optional.ofNullable(map.get("model"))
        .filter(value -> !value.isEmpty())
        .ifPresent(value -> { ... });
Andrew Tobilko
  • 48,120
  • 14
  • 91
  • 142
  • 2
    That's even more complex than the OP approach – developer_hatch Nov 08 '17 at 23:01
  • Np, also you might want to use `map.getOrDefault` in case the key does not exist, otherwise you will ironically get `NullPointerException` in that `filter` – smac89 Nov 08 '17 at 23:05
  • 2
    `Objects.isNull(map)` is more wordy than `map == null`. – Klitos Kyriacou Nov 08 '17 at 23:09
  • @smac89 `Optional#filter` will only apply the `Predicate` function if it's present and else return an **empty** `Optional` instead. So it should be fine, I think. See [Optional#filter](https://docs.oracle.com/javase/8/docs/api/java/util/Optional.html#filter-java.util.function.Predicate-). – Zabuzard Nov 08 '17 at 23:09
  • @KlitosKyriacou, yes, but with the static import it is more readable to me – Andrew Tobilko Nov 08 '17 at 23:11
0

If you've declared map as showing in your sample code then it won't be null and you don't need to check for it. If you want to make sure then add an assertion:

assert map != null;

Given you are testing for empty strings, a possible approach is to use the empty string as a default if the key is not present:

if (!map.getOrDefault("model", "").isEmpty()) {
    ...
}

I think it's a shame that there was no method added to Map that returns an Optional rather than a null if the key isn't present. Something like:

map.getOptional("model").filter(v -> !v.isEmpty()).ifPresent(v -> {
    ...
}

While optionals have been added there's been little rework of older APIs to deprecate methods that return null to mean "not present".

sprinter
  • 27,148
  • 6
  • 47
  • 78
  • The problem with `!map.getOrDefault("model", "").isEmpty()` is that it will throw a NPE if the map contains the mapping `"model" -> null`, while the original OP's code prevents that. – Alexis C. Nov 08 '17 at 23:31
  • That's true. I read OP's code as detecting absent keys rather than null values. If the map shouldn't hold null values then this still seems a reasonable solution. But I agree that it doesn't protect against all NPEs. – sprinter Nov 08 '17 at 23:37
0

You can also try containsKey method. For example:

HashMap<String, String> map = new HashMap<String, String>();
map.put("model", "test");

 if(map.containsKey("model")){
   //some logic
 }
Hema LM
  • 71
  • 1
  • 5