1

I have the following code which works on two values viz. a List of String and a String. Both the List and String can be null.

List<String> myStringList = getMyStringList(); // may return null
String myString = "abc"; // may be null

List<String> finalStringList = Optional.ofNullable(myStringList)
            .map(strings -> strings.stream()
                    .filter(obj -> StringUtils.isEmpty(myString)
                            || myString.equals(obj))
                    .collect(Collectors.toList()))
            .orElseGet(ArrayList::new);

Idea is: If myStringList is null, then result in an empty ArrayList. If myString is null, then return myStringList as is. If myString is not null, then loop through the list and return all items matching myString.

Problem is, with solution above, I still loop through the whole list (strings.stream) even when myString is null.

Assuming myStringList is very big, how can I avoid looping the list for the case when myString is null?

That is,

1) do a null check on the list. If null, return a new empty list.

2) do a null check on myString. If null, return the list as is WITHOUT LOOPING.

3) if myString is not null, loop through the list and return the filtered list.

Naman
  • 27,789
  • 26
  • 218
  • 353
Vicky
  • 16,679
  • 54
  • 139
  • 232
  • 2
    why you are not using simple if check. – roottraveller Nov 06 '19 at 05:09
  • 1
    Best solution: fix `getMyStringList()` to return an empty list instead of `null`. – Ole V.V. Nov 06 '19 at 05:11
  • 1
    Related and a very good read: [How to best create a Java 8 stream from a nullable object?](https://stackoverflow.com/questions/29406286/how-to-best-create-a-java-8-stream-from-a-nullable-object) (I suggest that this question is partially a duplicate of that one) – Ole V.V. Nov 06 '19 at 05:18
  • @Vicky The question would be really clear if you could draw the expectation with all 4 possible combinations of `myString` and `myStringList` 's presence and absence. – Naman Nov 06 '19 at 09:19

4 Answers4

2

You could use a ternary like,

List<String> finalStringList = myString == null ?
        Collections.emptyList()
        : Optional.ofNullable(myStringList)
        .map(strings -> strings.stream()
                .filter(obj -> StringUtils.isEmpty(myString)
                        || myString.equals(obj))
                .collect(Collectors.toList()))
        .orElseGet(ArrayList::new);

Or a basic if and default to an empty list. Like,

List<String> finalStringList = Collections.emptyList();
if (myString != null) {
    finalStringList = Optional.ofNullable(myStringList)
            .map(strings -> strings.stream()
                    .filter(obj -> StringUtils.isEmpty(myString)
                            || myString.equals(obj))
                    .collect(Collectors.toList()))
            .orElseGet(ArrayList::new);
}
Elliott Frisch
  • 198,278
  • 20
  • 158
  • 249
  • So you reckon I will have to use a ternary.. I thought of that.. but wanted to be sure if that is the only way and it can not be done using Streams and Optional.. – Vicky Nov 06 '19 at 05:14
  • The `if` does not use a ternary; so you don't "have to use a ternary". But you can. – Elliott Frisch Nov 06 '19 at 05:14
  • 1
    As this requirement says: "2) do a null check on myString. If null, return the list as is WITHOUT LOOPING." than this `Collections.emptyList()` should be rather `myStringList` – lczapski Nov 06 '19 at 10:43
1

This may work for you (with Java 9+):

List<String> result2 = Optional.ofNullable(myString)
  .filter(Objects::nonNull)
  .map(mystr -> Optional.ofNullable(myStringList)
                        .filter(Objects::nonNull)
                        .or(() -> Optional.of(new ArrayList<>()))
                        .stream()
                        .flatMap(List::stream)
                        .filter(mystr::equals)
                        .collect(Collectors.toList())) //ArrayList not guaranteed
    .orElse(myStringList);

If you're using Java 8, the following should work, but it looks a little harder to read:

List<String> result = Optional.ofNullable(myString)
  .filter(Objects::nonNull)
  .map(mystr -> Optional.ofNullable(myStringList)
                .filter(Objects::nonNull)
                .map(list -> list.stream()
                            .filter(mystr::equals)
                            .collect(Collectors.toList()))
                .orElseGet(ArrayList::new))
  .orElse(myStringList);
ernest_k
  • 44,416
  • 5
  • 53
  • 99
  • I am on Java 8. But your solution with Java 8 is not working. – Vicky Nov 06 '19 at 07:10
  • It throws NullPointerException when myStringList is null. – Vicky Nov 06 '19 at 07:20
  • @Vicky. No, I get an empty list when I test it. You may need to share your snippet. – ernest_k Nov 06 '19 at 07:23
  • added snippet in the answer – Vicky Nov 06 '19 at 07:43
  • @Vicky Your requirements stated * If `myString` is null, then return `myStringList` as is*. In your example, `myString` is null, and therefore the result is null. But you're calling `System.out.println(result.size());` without checking if `result` is null. That explains your NPE. Right? – ernest_k Nov 06 '19 at 07:47
  • ok. my bad. I should have worded better... What I meant was, the null check for list is already done.. so we proceed only when list is not empty.. so the point where I say "as is" null check on list has already being done, and the list is not empty... – Vicky Nov 06 '19 at 07:49
  • @Vicky Not too sure what you mean. In this code, if `myStringList` is null, the inner stream won't run. That's the behavior of `Optional.map`. – ernest_k Nov 06 '19 at 08:01
0

Other solution

List<String> myStringList = Arrays.asList("abc", "aef"); // may return null
String myString = "abc"; // may be null

Map<Predicate, Supplier<List<String>>> predicateMap = Map.of(
      (v) -> myStringList == null, () -> new ArrayList<>(), // do a null check on the list. If null, return a new empty list.
      (v) -> myString == null, () -> myStringList, // do a null check on myString. If null, return the list as is WITHOUT LOOPING.
      (v) -> true, () -> myStringList.stream() // if myString is not null, loop through the list and return the filtered list.
                .filter(myString::equals)
                .collect(Collectors.toList())
      );
List<String> outputList = predicateMap.entrySet().stream()
      .filter(p -> p.getKey().test(null))
      .findFirst().get().getValue().get();

System.out.println(outputList);
lczapski
  • 4,026
  • 3
  • 16
  • 32
0

Personally, I think the code by if-else will be much more readable/maintainable/efficient:

if (myStringList == null || myStringList.size() == 0) {
    return Collections.emptyList();
} else if (StringUtils.isEmpty(myString)) {
    return new ArrayList<>(myStringList); // or return myStringList;
} else {
    return myStringList.stream().filter(e -> myString.equals(e))
                                .collect(Collectors.toList());
}

Okay, if you really want to avoid writing a few lines of code, try my library: abacus-common

return N.isNullOrEmpty(myString) ? N.newArrayList(myStringList) 
                                 : N.filter(myStringList, e -> myString.equals(e));
user_3380739
  • 1
  • 14
  • 14