4
\$\begingroup\$

Between the two implementations, which one is better?

    import org.apache.commons.lang3.stream.Streams;

    public static <T> T firstNonNull(final T... values) {
        return Streams.of(values).filter(Objects::nonNull).findFirst().orElse(null);
    }

or

    public static <T> T firstNonNull(final T... values) {
        if (values != null) {
            for (final T val : values) {
                if (val != null) {
                    return val;
                }
            }
        }
        return null;
    }

The context is that of a general-purpose utility class intended for use throughout the application. Indeed, performance is one concern.

\$\endgroup\$

3 Answers 3

Reset to default
6
\$\begingroup\$

Your use of streams is good.

It's idiomatic to format this as multiple lines, which makes it even better.

    public static <T> T firstNonNull(final T... values) {
        return Streams.of(values)
            .filter(Objects::nonNull)
            .findFirst()
            .orElse(null);
    }
\$\endgroup\$
3
\$\begingroup\$

In the for loop code, it is explicitly clear how the first non-null value is chosen. We can clearly see that the loop terminates immediately as soon as a non-null is found. The loop does not continue. The code is simple and well-structured.

The Streams version makes the assumption that it is doing the same efficient looping under the hood as your for loop does. If you use it frequently, then you already know its algorithm; otherwise, you need to read the documentation. Although it is one long(-ish) line of code, it is quite readable.

The reason we use code libraries is to avoid re-inventing the wheel. This assumes the library is efficient, well-tested and well-documented.

Documentation

It couldn't hurt to add a short comment to both functions describing the efficeincy.

Performance

You need to benchmark the 2 versions of code to see which performs better using realistic data sets, especially data that you expect to encounter in your application.

\$\endgroup\$
2
  • \$\begingroup\$ From where I copied the code, both methods are well-documented with examples. I stripped it because I am interested in the implementation. \$\endgroup\$ Commented 9 hours ago
  • \$\begingroup\$ @FlorianF: This is not your code? \$\endgroup\$ Commented 5 hours ago
3
\$\begingroup\$

The examples do not implement the same functionality. The first one throws a NPE when called as firstNonNull((Object[]) null). Allowing null arrays and lists is a bad practice anyway so I would remove the null check from the latter and replace it with non-null requirement

for (final T val : Objects.requireNonNull(values)) {

and likewise change the stream version to

Stream.of(Objects.requireNonNull(values))

Regarding which one is better: neither. The code is inconsequential. Both versions fulfill their purpose.

You did not explicitly ask for performance review, but since it is in the tags I should post this link about performance optimizations that I have in my profile: https://ericlippert.com/2012/12/17/performance-rant/ . There are other links there too which you should read.

\$\endgroup\$
4
  • \$\begingroup\$ Actually I don't think you can pass null to either methods. Anyway, I used Strams.of which is the apache implementation, and it returns an empty Stream if given a null value. Indeed, performance is a concern. \$\endgroup\$ Commented 9 hours ago
  • \$\begingroup\$ Yes, you can pass a null array to a varargs method with the way I showed in the answer. And if you use a specific library, you should include the import statement in the code. \$\endgroup\$ Commented 9 hours ago
  • \$\begingroup\$ You are right. I assumed that with this syntax values would be assigned an array of Object[] with null being the first and only element. \$\endgroup\$ Commented 7 hours ago
  • \$\begingroup\$ Stream.of(Objects.requireNonNull(xxx)) is pointless here, as Stream.of(xxx) does same check + throw internally with improved message (on later JDKs) "Cannot read the array length because "array" is null". A little nicer than unspecified NPE. (Not sure if this applies to Streams as used in question). \$\endgroup\$ Commented 6 hours ago

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.