CODEWITHSUNDEEP
javaconditional-statementsreturnlogical-operators
jhlu87
jhlu87

Reputation: 4019

java using logical operators instead of if-else if for return

I have the following code in an equals method. 

public boolean equals(Object o){
    if (o == null) return false;
    if (o == this) return true;
    if (!(o instanceof Vertex)) return false;
    return ((Vertex) o).label().equals(label);
}

My IDE highlights the if statement and wants me to basically do this

public boolean equals(Object o){
    return (o != null) && ((o==this) || ((o instanceof Vertex) && ((Vertex) o).label().equals(label);
}

I've been told that the compiler is generally smart enough to optimize and that in general one should code for readability. So, clearly the second code sample is not as easy to read as the first. Is my IDE just being annoying or is there some actual performance merit to doing it that way?

Upvotes: 2

Views: 2822

Answers (6)

Shaunak
Shaunak

Reputation: 18028

Why not test the performance your self? Here is a sample:

public static long timeIt(Runnable runnable) {
    long start = System.nanoTime();
    runnable.run();
    long end = System.nanoTime();
    return end - start;
}

will time the run method for you. So you can call it like this:

timeIt(new Runnable() { public void run() { OPERATION_TO_BE_TIMED; }})

Upvotes: 0

peterm
peterm

Reputation: 1053

The performance benefit of the version suggested by your IDE is virtually none. Furthermore, you are right that JVM (not only compiler) can do a lot of optimizations.

As others pointed out, try to reach for readability. The code you write is read by human beings and compiled, modified and optimized by Java compiler and virtual machine.

Btw. a short hint: your code can be optimized a little more like this (leaving out the condition for equality to null, since o is guaranteed to be instance of Vertex after the second condition):

public boolean equals(Object o){
    if (o == this) return true;
    if (!(o instanceof Vertex)) return false;
    return ((Vertex) o).label().equals(label);
}

Upvotes: 1

James Black
James Black

Reputation: 41858

The benefit to doing it this way is that if you inline the code then you can assign it easily to a boolean, but you could also use a ternary operator to make it more readable, which will be more readable than the optimized version, but would have the same functionality, and may help you to simplify coding later if you learn how to do it.

public boolean equals(Object o){
    return o == null || !(o instance of Vertex) ? false :
       o == this ? true :
       ((Vertex) o).label().equals(label);
}

This should still be readable.

I haven't tried to compile this, some extra parenthesis may be needed, but I am going for a concept not for something you can copy & paste.

Upvotes: 0

Andrew White
Andrew White

Reputation: 53516

First, only optimize if you know it's a bottleneck, else code for readability.

You can check the byte code to see but I suspect they are pretty close if not exactly the same. Even if there are slight differences in bytecode, I've seen the JIT compiler optimize stuff down to where there is no difference. You can always do performance tests to be sure.

Upvotes: 2

Corey Sunwold
Corey Sunwold

Reputation: 10254

Don't try to out smart the compiler. You are writing code for a person to read. Let the compiler worry about converting it to machine readable format.

Upvotes: 0

Ray Toal
Ray Toal

Reputation: 88378

Most IDEs are configurable and allow you to specify the kinds of style warnings it outputs. You are correct that there is no benefit to doing what the compiler suggests in terms of the code that is generated. Compilers should indeed be able to optimize these kinds of things.

Upvotes: 1

Related Questions