3

I need to do a series of null checks ( nested null-checks ) to get an array of strings like below

String[] test;
if(CollectionUtils.isNotEmpty(checkList)){
    if(MapUtils.isNotEmpty(checkList.get(0))){
        if(StringUtils.isNotBlank(checkList.get(0).get("filename"))){
            test = checkList.get(0).get("filename").split("_");
        }
    }
}

Is there a better way, maybe using Java8 Optional, to perform these kind of nested checks? I unsuccessfully tried to use Optional with flatmap / map.

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
adbdkb
  • 1,897
  • 6
  • 37
  • 66
  • I thought `!checkList.isEmpty()` will return an NPE. I thought I would have to do `checkList!= null && !checkList.isEmpty()`, so I used the other library. So for this checking, must I have the nested `if()` statements? – adbdkb May 21 '19 at 12:47
  • 1
    Something like `return Optional.ofNullable(checkList) .filter(l -> !l.isEmpty()) .map(l -> l.get(0)) .filter(m -> !m.isEmpty()) .map(e -> e.getOrDefault("filename", "").split("_")) .orElse(new String[10]);` which is not good unless you know, why those hardcoded values are floating in. – Naman May 21 '19 at 12:54
  • 1
    Don’t let collection references be `null` in the first place. Then, you don’t need 3rd party methods performing `null` checks. – Holger May 21 '19 at 14:28

3 Answers3

4

You could use a long chain of Optional and Stream operations to transform the input step by step into the output. Something like this (untested):

String[] test = Optional.ofNullable(checkList)
    .map(Collection::stream)
    .orElseGet(Stream::empty)
    .findFirst()
    .map(m -> m.get("filename"))
    .filter(f -> !f.trim().isEmpty())
    .map(f -> f.split("_"))
    .orElse(null);

I'd strongly encourage you to stop using null lists and maps. It's a lot better to use empty collections rather than null collections, that way you don't have to have null checks all over the place. Furthermore, don't allow empty or blank strings into your collections; filter them out or replace them with null early, as soon as you're converting user input into in-memory objects. You don't want to have to insert calls to trim() and isBlank() and the like all over the place.

If you did that you could simplify to:

String[] test = checkList.stream()
    .findFirst()
    .map(m -> m.get("filename"))
    .map(f -> f.split("_"))
    .orElse(null);

Much nicer, no?

John Kugelman
  • 349,597
  • 67
  • 533
  • 578
  • Thanks. I agree with not using null lists and maps - but... I am receiving this data from some other part of the system hence do not have control over it. I will try the above with that assumption and see if it works for me – adbdkb May 21 '19 at 13:45
  • The first one worked well. For the second one, I am trying to work with the team that is generating the list to see if they can make the change to ensure to send empty lists or maps instead of null.. The second one also worked on the testcase where i ensured not to have nulls in the data received. – adbdkb May 21 '19 at 15:36
1

Don't nest the ifs, but just unwrap and invert them:

String[] defaultValue = // let this be what ever you want

if(checkList == null || checkList.isEmpty()) {
    return defaultValue;
}

Map<String, String> map = checkList.get(0);
if(map == null || map.isEmpty()) {
    return defaultValue;
}

String string = map.get("filename");
if(string == null || string.trim().isEmpty()) {
    return defaultValue;
}

return string.split("_");

Though this only works when you wrap this extraction logic in a method:

public static String[] unwrap(List<Map<String, String>> checkList) {
    ...
}
Lino
  • 19,604
  • 6
  • 47
  • 65
0

If checkList is null, it will throw null pointer exception on CollectionUtils.isNotEmpty(checkList). Also use in-built empty checker. Better you should code

        if (null != checkList && !checkList.isEmpty() 
                && null != checkList.get(0) && !checkList.get(0).isEmpty()
                && StringUtils.isNotBlank(checkList.get(0).get("filename"))) {

            test = checkList.get(0).get("filename").split("_");

        }
mate00
  • 2,727
  • 5
  • 26
  • 34
Chirag Shah
  • 353
  • 1
  • 13