Improving the little things in Java code

CPColinSenior Java Architect
EE Senior Architect
Published:
If you write a lot of code, either professionally or as a hobby, it's almost inevitable that somebody else will read your code at some point. I spend a lot of time reading code in the Java runtime library, in third-party libraries, and in old projects I've worked on. I tend to notice patterns of code that could be just a little bit better, even in code I've written. It occurred to me that, when reading some code, these patterns can taint my perception of the quality of the overall product. When evaluating a third-party library, I might pass it up, if a large portion of it is written in a less-than-pleasing way.

When you apply for a job that requires a sample of your work, your potential employer could pass you up, for the same reason.

Code that works well is good code. Code that looks nice is even better. Here are some patterns that come up a lot, with some suggestions for how to improve them.
 

Iterating over a map


This pattern comes up a lot, when iterating over an instance of Map:
 
Map<K, V> map;
                      
                      for (K key : map.keySet())
                      {
                         doSomethingWithKey(key);
                         doSomethingWithValue(map.get(key));
                      }

Open in new window


Here's the problem, though: this code is calling Map.get() for each key. This code should be iterating over the entries in the map, not the keys. Fortunately, Map.entrySet() gives us exactly what we need:
 
Map<K, V> map;
                      
                      for (Map.Entry<K, V> entry : map.entrySet())
                      {
                         doSomethingWithKey(entry.getKey());
                         doSomethingWithValue(entry.getValue());
                      }

Open in new window


This lets us skip doing the extra map lookups.
 

Initializing variables too soon


Sometimes, code will initialize a variable to a value, then immediately set it to a different value:
 
Object value = DEFAULT;
                      
                      if (condition)
                         value = new Object();

Open in new window


It's a minor point, but that code could be written like this, instead:
 
Object value;
                      
                      if (condition)
                         value = new Object();
                      else
                         value = DEFAULT;

Open in new window


This code has a similar issue:
 
Object value = DEFAULT;
                      
                      try
                      {
                         value = new Object();
                      }
                      catch (Exception e) {}

Open in new window


It could be written like this:
 
Object value;
                      
                      try
                      {
                         value = new Object();
                      }
                      catch (Exception e)
                      {
                         value = DEFAULT;
                      }

Open in new window


This also prevents an "Empty block must be documented" warning in Eclipse.
 

Computing a value too soon


Slightly worse than initializing a variable too soon is computing the value of a variable too soon. Consider the following code:
 
Object value = computeValue();
                      
                      if (condition)
                         return computeSecondValue(null);
                      else
                         return computeSecondValue(value);

Open in new window


If the first branch of the conditional executes, the computed value goes to waste. This code could work like this:
 
if (condition)
                         return computeSecondValue(null);
                      else
                         return computeSecondValue(computeValue());

Open in new window

 


The more code you read, the faster you'll notice patterns like this. The more code you write, the more often you'll avoid patterns like this. None of these patterns actually causes any bugs, but all of them could affect how others perceive the quality of your code.
4
2,757 Views
CPColinSenior Java Architect
EE Senior Architect

Comments (5)

CERTIFIED EXPERT

Commented:
Nice tips CPColin.  I totally agree with the map one and the computing a value too soon one.

The variable initialization, for me is a bit incomplete.  Perhaps because these days I spend a lot of time trying to write immutable objects.  So I'd prefer this pattern over any of those shown:

    Object value = (condition) ? new Object() : DEFAULT ;

That's because it supports this version:

    final Object value = (condition) ? new Object() : DEFAULT ;

which shows up a lot if you're trying to make your objects immutable (so they perform better in multi-threaded environments or in Java 8's functional language features).

But aside from that small point, very nice advice.

Doug
CPColinSenior Java Architect

Author

Commented:
This style works fine in Eclipse, at least, for me:

final Object value;

if (condition)
   value = new Object();
else
   value = DEFAULT_VALUE;

Open in new window


Looks like the compiler is able to figure out that the final variable is only ever going to be assigned a single value.
CERTIFIED EXPERT

Commented:
Interesting - I would not have expected that to work.  But I guess it's safe because if you attempted:

final Object value;

System.out.println(value) ;

if (condition)
   value = new Object();
else
   value = DEFAULT_VALUE;

Open in new window


to access the variable too soon, you'd get the "may not have been initialized" error.

Still, not sure I'd prefer the more expanded pattern myself over the more definitive "it's only initialized once because there's only 1 assignment statement" but you make a good point that it's more of a preference than a requirement.

Doug
CERTIFIED EXPERT
Most Valuable Expert 2013

Commented:
But given that the article is essentially about practicing best coding practices... is a trick that works well, but is still a trick a best practice?
CPColinSenior Java Architect

Author

Commented:
It's not really a trick; Doug's version is just a single-line version of the example code I suggested. If my code did some complex computations in each branch of the conditional before initializing the variable, we wouldn't be able to condense it down to one line. As it is, though, both my version and Doug's version are perfectly valid ways of performing the same operation. (I'd even bet that the bytecode came out the same.)

Have a question about something in this article? You can receive help directly from the article author. Sign up for a free trial to get started.