views:

274

answers:

7

A colleague of mine sets reference to null in finally blocks. I think this is nonsense.

public Something getSomething() {
    JDBCConnection jdbc=null;
    try {
        jdbc=JDBCManager.getConnection(JDBCTypes.MYSQL);
        ...
    }
    finally {
        JDBCManager.free(jdbc);
        jdbc=null; // <-- Useful or not?
    }
}

What do you think of it?

+7  A: 

You are correct, jdbc is a local variable so when the getSomething() method returns jdbc will be out of scope and eligible for garbage collection which is effectively the same as setting it to null. So there is no point in setting a variable to null when it is out of scope in the next line of code.

It is good practice to limit variables to the smallest scope that is needed, e.g. if you only need a variable inside a for loop then declare it in the for loop and it will be eligible for garbage collection when the code exits the for loop. This, as well as reducing the complexity of your methods reduces the need to even set local variables to null at all and as a benefit your code becomes more modular, easier to read and maintain.

krock
+2  A: 

Yes, this is pretty much nonsense. The motivation is usually to "help" the garbage collector, but this is not real help at all since the reference is cleared anyway. Although it does no harm either, at least not for the VM - your eyes and sanity are a different matter.

However, the example doesn't return Something. If the example is incomplete, and there are in fact statements after the finally block, then setting jdbc to null can serve as a deterrent to use, and the NPE that would occur immediately informs of any use after the finally block.

mdma
+4  A: 

Since it is a local variable, it will get out of scope anyway. It's nonsense.

If it was an instance variable (member variable) of a long lived object, it could be useful since it otherwise might prevent the garbage collector from disposing the object.

aioobe
A: 

If there would be more code after the finally block instead of just ending the method, it might help the garbage collector clean it up.

Andrei Fierbinteanu
More to that - it would prevent programmers from using jdbc after it has been free'd.
Andreas_D
No, it won't help the gc clean in up. The JVM spec is quite clear that only references which contribute to further calculations are reachable, so if you null it and don't get a NPE, then nulling it has no effect. If you null it and would get a NPE, you've broken the code. It's easier to put an extra {} around the declaration and use, or better refactor its use out to a separate method, to find out whether it is used at compile time.
Pete Kirkham
jdbc is already null.
Steve Kuo
'if you null it and don't get a NPE'. What's that supposed to mean? You can't get an NPE setting something to null.
EJP
+1  A: 

In this special case, technically spoken, it's really useless. When the method returns, jdbc doesn't exist anymore and will not hold a reference to a connection, so it will not affect garbage collection.

It's a question of coding style. There's a small advantage if one day you add more code after the finally block. Then it's immediatly obvious that you can't use jdbc anymore because it has been free'd by the JDBCManager.

So yes, it is good practise to nullify references to disposed resources.

Andreas_D
+1 for showing the _programmer_ that a given reference is invalid.
Thorbjørn Ravn Andersen
whoever downvoted that answer - I'd be happy to know your reason. Really!
Andreas_D
+1  A: 

as already written, in this case it is useless, because the method ends after the finally.
I would do it, if there is code following the try-finally, to eventually prevent its usage. And there are (very rare) situations where it can help.
Have a look at this article: Java Memory Puzzle

Carlos Heuberger
A: 

then he should set all local variables to null in all methods before return.

JVM probably will optimized the line off anyway, so it has no runtime effect whatsoever.

irreputable