Home » Java » Core Java » Ineffective Java

About Ashley Frieze

Ashley Frieze
Software developer, stand-up comedian, musician, writer, jolly big cheer-monkey, skeptical thinker, Doctor Who fan, lover of fine sounds

Ineffective Java

Perhaps I can be replaced by a robot for code review. There are some bits of feedback I find myself giving over and over again. Here are some of my least favourites:

General Code Structure

Drop The Else

When if ends in return the else is superfluous and creates unnecessary indentation.

01
02
03
04
05
06
07
08
09
10
11
if (foo) {
   return bar;
} else {
   return baz;
}
 
// should be replaced by
if (foo) {
   return bar;
}
return baz;

Array -> List -> Stream

1
2
3
4
5
6
List< ... > list = Arrays.asList(someArray);
list.stream(...)
 
// should be replaced by
 
Arrays.stream(someArray)

Test Code

Before is a Heavy Initializer

We use a @Before method to set up complex objects, often where we need to do processing to calculate what the class instance member needs to have in it. At the other end of the spectrum, it’s overkill:

01
02
03
04
05
06
07
08
09
10
11
12
13
14
15
16
17
18
// this is part 1 of two
private MyService myService;
 
@Before
public void before() {
    // now initialize
    myService = new MyService().init(123);
}
 
// the above code can be expressed in the initializer
// and is simple to read there...
// if it threw exceptions or needed some more complex
// set up, it wouldn't be
 
// it's in one clear place where we know where to
// find it
private MyService myService = new MyService()
    .init(123);

Test Throws

01
02
03
04
05
06
07
08
09
10
11
12
@Test
public void someTest()
    throws IOException, JsonException {
}
 
// never bother with multiple or specific exception
// throws in tests nobody cares and it's just noise
// the test runner will catch anything!
 
@Test
public void someTest() throws Exception {
}

AssertJ for Size

1
2
3
4
5
// long-winded
assertThat(list.size()).isEqualTo(2);
 
// should be
assertThat(list).hasSize(2);

AssertJ for Everything

The built in JUnit assertions are not as rich as those provided by AssertJ. As a minimum, I recommend using some form of assertThat, so you don’t end up using an assertion that’s a bit weak for the situation.

Your assertEquals Is The Wrong Way Around

60% of the time, when I review code with assertEquals in, the order is wrong. Hint: use AssertJ!!! JUnit is wrong on this one! We should read from left to right.

1
2
3
4
5
// wrong:
assertEquals(something.getFoo(), 123);
 
// it's expected IS actual
assertEquals(123, something.getFoo());

Mockito Static Imports

1
2
3
4
5
// this is not normal
Mockito.verify(mock).called();
 
// static import all mockito methods
verify(mock).called();

Mockito Times(1)

1
2
3
4
5
6
7
// this is a tautology
verify(mock, times(1)).called();
 
// look at what verify(mock) does internally
// replace with
 
verify(mock).called();

Published on Java Code Geeks with permission by Ashley Frieze, partner at our JCG program. See the original article here: Ineffective Java

Opinions expressed by Java Code Geeks contributors are their own.

(0 rating, 0 votes)
You need to be a registered member to rate this.
7 Comments Views Tweet it!
Do you want to know how to develop your skillset to become a Java Rockstar?
Subscribe to our newsletter to start Rocking right now!
To get you started we give you our best selling eBooks for FREE!
1. JPA Mini Book
2. JVM Troubleshooting Guide
3. JUnit Tutorial for Unit Testing
4. Java Annotations Tutorial
5. Java Interview Questions
6. Spring Interview Questions
7. Android UI Design
and many more ....
I agree to the Terms and Privacy Policy

7
Leave a Reply

avatar
4 Comment threads
3 Thread replies
0 Followers
 
Most reacted comment
Hottest comment thread
6 Comment authors
Mateusz GórskiSt1mpyChris Kesseltemple run 2Ashley Frieze Recent comment authors

This site uses Akismet to reduce spam. Learn how your comment data is processed.

  Subscribe  
newest oldest most voted
Notify of
Nils Weinander
Guest
Nils Weinander

The first example is even more succinct as

return foo ? bar : baz;

Ashley Frieze
Guest

In this trivial case, you’re right. Ternary expressions are a good solution to the most trivial cases, though extracting an explanatory function to name the calculation is often a good addition to it.

The point of the above patterns, which will grow on the source site (cited in the article) is about the structure of if statements in general. I’ve used one-liner examples to illustrate this.

temple run 2
Guest

The built in JUnit assertions are not as rich as those provided by AssertJ. As a minimum, I recommend using some form of assertThat, so you don’t end up using an assertion that’s a bit weak for the situation.

Chris Kessel
Guest
Chris Kessel

The first case I’ve seen mentioned before and is one I fervently disagree with. The else indicates it’s mutually exclusive. I shouldn’t have to look up further in the code for a return to figure that out. Further, it prevents errors. I’ve seen this bug many times in my career: if (something) { …a few setup lines… doSpecialThing(); // oops, should be return doSpecialThing() } return doOtherThing(); This will always doOtherThing() because of the bug in the “if” where the person forgot the return. doOtherThing() is a mutually exclusive path and if you use the else to indicate it, then… Read more »

Ashley Frieze
Guest

Interesting insight there, and I’ll probably incorporate some of what you’ve said into a future article, along with a footnote on the if statement. This will appear at the original site; I don’t know if they’ll replicate the updates here. I’d like to make three points in response to yours, though. 1. As you get to more mission-critical applications, you may need to add otherwise redundant programming styles to your code to add assurance. 2. If your methods are so long that you can’t see the structure at a glance (I’m taking 5 lines of code as a “large method”),… Read more »

Mateusz Górski
Guest
Mateusz Górski

100% Agree.

St1mpy
Guest
St1mpy

Article is crap! Please replace author by robot