What to Look for in Java 8 Code

Trisha Gee

In earlier blog posts we’ve talked about general things to look for during a code review. Recently I’ve been working a lot with Java 8, and I’m starting to have some opinions on what modern Java code should look like.  I’ve written about some of my Top Tips from an IntelliJ IDEA point of view, but I’d like to share these and more in the form of feedback you could give during a review of Java 8 code.

Just a quick note on the screenshots in this post: I’m (mostly) using the single page diff that’s the default view in an Upsource review (with white-space changes turned off). Code with a red background has been removed, code in green is the updated code and usually the code I’m referring to when I talk through the example.  Note that in this view, Upsource displays any inspection failures inline with the code that caused the failures.

Optional

I want to start, unusually, with Optional.  I know it’s not one of the “sexy” features, like Lambdas and Streams, but it’s definitely an area where a second pair of eyes on the code can yield benefits – as always, IntelliJ IDEA’s inspections configured in Upsource can help to some extent, but there are things an experienced developer should be able to spot that a tool will not.

Optional should only be used for return types
If you have your Optional inspections turned on, Upsource will flag this problem for you.

The simplest way to use Optional and not have it running amok all over your code base is to only use it in return types and have calling code deal with the empty case as soon as it can.

If, as a reviewer, you see an Optional value living longer than a handful of lines of code, it’s a good idea to ask “why?”.  Optional has a very simple job – to declare that something may be null, and allow the caller to cater for this.

You could argue that declaring Optional as a parameter is a good way of specifying that the parameter is allowed to be null.  But there are a number of issues with this: firstly, the Optional value is not only allowed to be empty, but could in fact be a null itself.  So stating the parameter is of type Optional doesn’t always negate the need for a null check; secondly, forcing the calling code to wrap a parameter is potentially introducing additional object creation, which could be bad for performance; thirdly, and most interestingly from a code review point of view, it can make code that calls the method ugly:

If this method is updated to just take a message, and a null check is added instead of ifPresent, the code is generally simpler for callers.

Similarly, it’s tempting to use Optional for fields that we know are allowed to be null, but introducing this Optional either gives us very little value (see the image below), or at its worst starts to leak all over the code if we provide getters that return the Optional values.

Don’t call get() without checking the value is present
If you call get without checking if there’s a value there, you’ll run into a NoSuchElementException, which is little better than the NullPointerException this feature is supposed to avoid. Again, this is something that’s flagged up by your inspections.

Look for the most elegant solution
This is something for a code reviewer and not automatically flagged.  Yes, you can simply use isPresent to check for a value before doing anything:

But it’s more elegant to use ifPresent with a lambda expression.

If there’s a value you want to use if the Optional is empty, you could use an if/else

But the best solution is using orElse

If, like in this example, you’re using a method to create or fetch this value (rather than a field or variable), you’re better off expressing this with orElseGet

It adds a tiny bit more code, via the lambda expression, but it means the method is only called if the value really is empty, so if this method is relatively expensive you won’t incur the cost of calling it unless you really need to. As you can see, it may be possible to shrink this down further using a method reference

So bear in mind that while inspections can point out some obvious places for errors when you’re using Optional, code reviewers should be looking to see which pattern of use is appropriate when they see an Optional being used.

Using Lambda Expressions

Inspections can point us to places where existing code might be changed to use lambda expressions, so I’m going to largely ignore those and look at places where a human code reviewer can add value.

Keep it Short
It’s possible to set up static analysis to flag long methods, including long lambda expressions. But generally a code reviewer is probably in the best position to decide if a lambda is “too long” according to whatever feels right for the team. This example is a lambda expression spanning lines 1466-1480, it’s so long you don’t necessarily notice it’s even a lambda.

My personal preference is for single-line lambda expressions, and where possible I like to use descriptive method names which make sense as a method reference.  Here, the code has been updated to replace the two-line lambda expression with a method and method reference:

Be Explicit
You lose a lot of written type information with lambda expressions.  You’ll need to decide as a team what your preference is around readability. Some may prefer to be very explicit, and always chose to include parameter types in lambda expressions, and maybe even include the return statement:

I prefer to keep code short but readable, so I prefer descriptive parameter names but to minimise other “noise” – removing unnecessary types and brackets:

If as a team you are new to Java 8, you might pick the first option but move towards eliminating boilerplate as you become more comfortable with the idioms. Remember consistency is key to readability, so make a decision and stick with it, and if you do decide to change that decision, have a policy on when/how to update code to the newer standards.

Designing for Lambda Expressions

Using lambda expressions with APIs like the Streams API (more on that later) is probably going to be quite common when coding with Java 8.  I believe that designing methods that take lambda expressions is probably going to be less common, especially for those of us who are used to coding in older versions of Java. But if you are designing for lambdas, here are some suggestions.

Do you really need a lambda expression?
The temptation, when you start using a new thing, is to use it everywhere. You may be tempted to pass lambdas around all over the place, into methods that really don’t need behaviour as a parameter. As a code reviewer, if you see a new method that takes a lambda parameter, you should ask yourself if it’s really necessary.

The original code (on the left) without a lambda was shorter and perfectly readable. Remember it’s not the method itself that should benefit from taking a functional parameter, it’s the caller of the method. If using a lambda expression when calling the method is clumsy or difficult to understand, the method probably shouldn’t have a functional parameter.

Use existing functional interfaces
If a method (or more accurately, callers of a method) really will benefit from having a functional parameter, code reviewers should be checking that the parameter type is, where possible, one of the existing functional interfaces. As we use Java 8 more, we’ll know what to expect when using interfaces like Supplier and Consumer, so introducing new interfaces could be less understandable.

When you see a new functional interface being introduced, you should check to see if there’s something in the java.util.function package that will do the trick.

Add @FunctionalInterface to your functional interface
If you really really do need a new functional interface, either because there isn’t one that matches the inputs and outputs required, or because you think the code is more understandable with an interface with a specific descriptive name, make sure it’s tagged with @FunctionalInterface.  This lets other developers know it can be implemented as a lambda expression, tells the compilers what to expect from this interface (specifically a Single Abstract Method), and IDEs like IntelliJ IDEA can use this to suggest it as a type when introducing functional parameters.

Behaviour on Interfaces

Interfaces now support default methods and static methods. These two types of methods serve two different purposes. Default methods are good for providing a default implementation of a method, and are particularly useful for adding a new method to an existing interface without breaking compilation of any classes that implement this interface, and without having to change all implementations.  Static methods can be used like functions, and can provide simple functionality that could be used, for example, within lambda expressions or as a method reference.  The new static methods on Comparator are good examples of this pattern. This can replace the previous pattern of having an interface, like Collection, and a companion class with helper utilities, like Collections.

Generally speaking, a developer is not going to look for behaviour on an interface, so the existence of default or static methods may be forgotten, or may be confusing for developers new to Java 8.  I would be wary of using either of these features unless it’s absolutely the right tool for the job.

Default methods are useful, but don’t go crazy
This is a good place for a code reviewer, as it’s not something that can be checked automatically.  Default methods are good for providing a, well, default implementation of something that implementing classes can choose to override if they wish.

But an interface filled with default methods may suggest a missing domain concept – if an interface needs that much behaviour, maybe it should live on a full-blown class instead. And remember that Java classes can implement multiple interfaces, so if two interfaces have default methods with the same name, these can potentially clash.

As a code reviewer, you want to ensure a default method really is needed, and really is a default implementation of the functionality.

Perhaps a static method would be better
Default methods are not the only way to add behaviour to an interface, they can now have static methods as well. If you see default methods on an interface, you may want to question whether this really is behaviour that can or should be over-ridden, or whether this is a valid standalone method that could be static.

Above, the first default method looks like it might be something that implementations want to override.  But the second could stand on its own as a static method:

Again, if you see a lot of static methods floating around you might want to question if, in fact, they should belong to a full-blown class.

Comparator

Assume there’s an easier way than specifying a Comparator implementation
Comparators have had a really nice facelift in Java 8, but this is an area that many may not be aware of.

If you see an implementation of a Comparator defined, firstly you’ll want to check this is now a lambda expression and not an anonymous class

But it doesn’t stop there.  Comparator, as mentioned above, has a number of new static methods, and these can make a developer’s life much easier

Check out comparing and comparingInt, for example, and note that the default method reversed is also useful.

When you combine the new methods with static imports and method references, the compare operation is suddenly much more readable than even the new lambda expression.

Streams

Have a convention and follow it
My personal preference is for lining up the dots of a stream operation. I think this is more readable, and it also makes it easier to debug, to comment out steps when trying things out, and to add steps where necessary.

I also prefer to use method references where possible (more on this later), use static imports if the method name is readable without the class name, and inline variables (the exception to this is where a temporary variable name aids readability).

Be aware of newer helper methods
Java 7 and Java 8 have both introduced static methods which are very helpful in the context of Streams.  We mentioned above that Comparator has the new comparingInt method, for example:

And there’s also Objects, a class with some useful methods:

Use method references
Method references are not to everyone’s taste, and if you thought the type information went missing with lambdas, with method references you’re not even sure how many parameters there are or whether it returns a value.  But getting to grips with this syntax not only means you have to type less, it can aid readability.  In both of the examples above, using a method reference makes it explicit what the expected behaviour is – the first, that you’re comparing on getTweetCount, the second that you only want values that are not null.

Use the Streams API or new Collections methods by preference
The Streams API is powerful, readable (although may take a bit of getting used to) and generally performs complex operations in a single pass of the data. Therefore stream operations are easier to understand in the code and potentially could perform better too. Unless you have specific performance requirements and a performance test to prove using the Streams API (or new collections methods like forEach), you may decide the newer idioms should be preferred, certainly in new code.

Of course, inspections can point out where these can be used, and an IDE like IntelliJ IDEA will automatically refactor these for you.

…but multiple stream operations are a sign of a smell
Of course, every recommendation comes with a caveat. Performing multiple stream operations on the same collection may suggest there’s a way to solve the problem that only needs a single stream operation.  After all, their power comes from performing everything in a single pass of the data.

Code like this should raise the question, is there a different way to approach this problem?

…but mixing idioms should raise questions
As you migrate code from older versions of Java to Java 8, you may choose to update existing code to use the new idioms where you find it. Sometimes you may find that having a mix of for loops and stream operations is confusing and adds little value.

If these can’t be collapsed to a single stream, this might be a place to suggest more aggressive refactoring.

…but use for loops when working with primitive arrays
Streams are almost definitely going to be too heavy-weight for iteration over a primitive array.  Even if nano-second level performance is not a key requirement of your system, sticking to simple for loops for iterating over arrays is likely to make sense from both a readability point of view and performance perspective.

Having said there, there are primitive streams too, and they have some nice functionality for things like ranges.

Although if you do use range, there are some gotchas to check here too, you need to understand if the range should be inclusive or exclusive.  The code above is not correct, if you want a range from one to five inclusive, you should use rangeClosed.

Parallel, is it really needed?
Parallel looks like a magic keyword to make your code go faster, however parallelising your code is not free. So the benefits of running your code across multiple CPUs needs to outweigh the code, and unless you’re working with big data sized code, often you won’t get any performance improvements simply by using parallel streams.

If you see parallel or parallel stream used in the code, I would want to see a corresponding performance test with production-like data to prove this is needed, and works faster.

Finally
As with all things code-style related, there really aren’t any hard and fast rules. The above are preferences that I personally have for working with Java 8 code, but you and your team may have different opinions.  We’d love to hear about any hints or tips you’ve picked up using Java 8!