2

I've got this method at the moment that I am trying to refactor.

public static boolean isEmpty(Object value) {
    boolean empty = false;
    if (value instanceof String && ((String) value).isEmpty()) {
        empty = true;
    } else if (value instanceof List && ((List<?>) value).isEmpty()) {
        empty = true;
    } else if (value instanceof String[] && ArrayUtils.isEmpty((String[]) value)) {
        empty = true;
    } else if (value == null) {
        empty = true;
    }

    return empty;
}

Is there an obviously better way to do this that I am missing?

I know I could put all the conditions on one if statement using chained || statements, but I don't really see how that is better than what I've got now.

LockieR
  • 370
  • 6
  • 19
  • for a list if (myList.isEmpty() ) instead of if ( isEmpty(myList)) – Stultuske Jul 08 '21 at 07:42
  • 1
    There isn't a significantly better way to do this. Only minor tweaks. Ultimately you need to do a sequence of tests. But in Java 17 you will have the option of using `switch` expressions to do this; see https://openjdk.java.net/jeps/406 – Stephen C Jul 08 '21 at 08:14
  • 1
    additionally to the answers below, the null check should be the first test, since if the value is null, no instanceof and cast is needed. – juwil Jul 08 '21 at 08:37
  • 2
    @juwil - I would leave that to the compiler. There is an implicit null check in `instanceof` ... and the JIT optimizer is good at eliminating unnecessary (e.g. duplicate) null checks of all kinds. – Stephen C Jul 08 '21 at 09:02
  • 2
    The better approach is to redesign your application, removing the need for such a horrible method. Why do you need to handle entirely different objects with a single method? What does `ArrayUtils.isEmpty((String[]) value)` do? Is it equivalent to `((Object[])value).length==0`? Or does it something obscure? What about other array types? – Holger Jul 08 '21 at 12:17

4 Answers4

6

In Java 17, you will be able to write this using a switch expression. According issue 8213076, the syntax will be:

public static boolean isEmpty(Object value) {
    return switch(value) {
      case String s -> s.isEmpty();
      case List l -> l.isEmpty();
      case String[] sa -> ArrayUtils.isEmpty(sa);
      default -> value == null;
    }
}

NB: This is a preview feature in Java 17. It could change or even be removed entirely in a later release.


(The original proposed syntax in JEP 406 was a bit different:

public static boolean isEmpty(Object value) {
    return switch(value) {
      case String -> value.isEmpty();
      case List -> value.isEmpty();
      case String[] -> ArrayUtils.isEmpty(value);
      default -> value == null;
    }
}

As you can see, the original version did not require a separate binding variable in each case.)

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
  • 2
    The syntax likely will be similar to the `instanceof` pattern matching of JDK 16. So you need a new identifier per case: `return switch(value) { case String s -> s.isEmpty(); case List l -> l.isEmpty(); case String[] a -> ArrayUtils.isEmpty(a); default -> value == null; };` – Holger Jul 08 '21 at 12:12
  • 2
    The `instanceof` pattern matching would work as well, i.e. `return value instanceof String s? s.isEmpty(): value instanceof List l? l.isEmpty(): value instanceof String[] a? ArrayUtils.isEmpty(a): value == null;` does already work in JDK 16. When using `--enable-preview`, it will even work in JDK 14 and 15. – Holger Jul 08 '21 at 12:33
  • 2
    @Holger To amplify Holger's comment: a new binding variable is necessary for each case, shown in his example as `case String s -> s.isEmpty();`. The type of `value` doesn't shift around in different places as it does with, say, Kotlin's flow typing. Also note that switch patterns are a _preview_ feature in Java 17, so they are not intended for production use, and things may change in the future. – Stuart Marks Jul 08 '21 at 16:12
  • This will be super helpful once it's implemented in Java 17, but unfortunately I am locked in to Java 8 at the moment. – LockieR Jul 08 '21 at 23:28
2

You can shorten this code by omitting the flag and directly returning the result of the checks for emptiness:

public static boolean isEmpty(Object value) {
    if (value instanceof String) {
        return ((String) value).isEmpty();
    } else if (value instanceof List) {
        return ((List<?>) value).isEmpty();
    } else if (value instanceof String[]) {
        return ArrayUtils.isEmpty((String[]) value);
    } else {
        return value == null;
    }
}

This could be considered an improvement of readability.

deHaar
  • 17,687
  • 10
  • 38
  • 51
  • All `else`s could be removed and the last `if`could be replaced by `return value == null` – Dietmar Höhmann Jul 08 '21 at 07:58
  • 1
    @khelwood totally right, the last one should be an `else` only and `return value == null` as the comment above states. – deHaar Jul 08 '21 at 08:05
  • 1
    Or just `return value instanceof String? ((String) value).isEmpty(): value instanceof List? ((List>) value).isEmpty(): value instanceof String[]? ArrayUtils.isEmpty((String[])value): value == null;` – Holger Jul 08 '21 at 12:24
  • Yes this seems like a solid idea. – LockieR Jul 08 '21 at 23:28
2

As mentioned in deHaar's answer, the code can be improved by using if-return combinations.

You should however generalize your code a bit more, e.g. support all array types, not just string arrays, and support all collection types, not just lists.

Similarly, String implements an interface named CharSequence, so support that instead. That way the code will support classes like StringBuilder and StringBuffer too.

public static boolean isEmpty(Object value) {
    if (value == null)
        return true;
    if (value instanceof CharSequence) // String, StringBuilder, StringBuffer, ...
        return ((CharSequence) value).isEmpty();
    if (value instanceof Collection) // List, Set, Queue, Deque, ...
        return ((Collection<?>) value).isEmpty();
    if (value instanceof Map)
        return ((Map<?,?>) value).isEmpty();
    if (value.getClass().isArray()) // All array types
        return (Array.getLength(value) == 0);
    return false;
}

Code was modified to to use pure built-in methods, i.e. to not rely on ArrayUtils.

UPDATED: Added support for primitive arrays.

The above implementation is a close match to the JSP EL empty operator:

The empty operator is a prefix operator that can be used to determine if a value is null or empty.

To evaluate empty A

  • If A is null, return true
  • Otherwise, if A is the empty string, then return true
  • Otherwise, if A is an empty array, then return true
  • Otherwise, if A is an empty Map, return true
  • Otherwise, if A is an empty Collection, return true
  • Otherwise return false
Andreas
  • 154,647
  • 11
  • 152
  • 247
  • 3
    Still doesn’t handle primitive arrays. Or `ByteBuffer`. Since this is an open end, generalizing the method is not going into the right direction. Rather, the OP should try to get rid of such a magic “empty, whatever that actually means” method. What about a class that implements `CharSequence` and `Collection` at the same time? – Holger Jul 08 '21 at 12:21
  • @Holger `ByteBuffer` doesn't have an `isEmpty()` method. – Andreas Jul 08 '21 at 17:09
  • I like this solution a lot, ideally I'd still like something cleaner, but for now this will suffice! – LockieR Jul 08 '21 at 23:29
  • 1
    Well, I agree with Holger that the cleanest solution is to get rid of this strange concept of emptiness that means different things for different objects / values. (Null and empty are not the same ...) – Stephen C Jul 08 '21 at 23:33
  • 1
    @Andreas right, currently `ByteBuffer` doesn’t have an `isEmpty()` method. Just like `CharSequence` hadn’t before Java 15. So is this method supposed to take the challenge of A) handle everything that conceptually contains elements or B) handle everything that currently has an `isEmpty()` method (to be adapted when changing)? If B then why does it handle arrays? – Holger Jul 27 '21 at 08:55
  • @Holger If you read the last part of the answer, we should be able to conclude that the method is supposed to mirror the JSP EL `empty` operator. Ok, so maybe I shouldn't have relaxed it to do `CharSequence` instead of `String`. ‍♂️ – Andreas Jul 27 '21 at 12:25
1

Java native utils gives you the abaility to check if Objects.isNull(value) which is not equivalent to empty. If you want to stick with native Java you are doing it right just beautify your code and that's it. However this method exists in springFramework ObjectUtils class regardless of the type of object :

import org.springframework.util.ObjectUtils;

public static boolean isEmpty(Object value) {
    boolean empty = false;
if (ObjectUtils.isEmpty(value)) {
empty= true
}
return empty;
}

Spring Framework doc : Class ObjectUtils

Hajed.Kh
  • 408
  • 3
  • 11