2
java version "1.7.0_75"

Hello,

Just wondering what is the prefered best practice comparing the 2 functions below.

The first one throws a NullPointerException that should be captured in the calling function. The second one just returns false if there has been a null pointer exception.

Throw an exception:

public void disconnect() throws NullPointerException {
    if(mClientConnection == null) {
        throw new NullPointerException("mClientConnection has an invalid reference");
    }

    if(mClientConnection.isConnected()) {
        mClientConnection.disconnect();
    }

    mClientConnection = null;
}

Just return true or false:

public boolean disconnect() {
    if(mClientConnection == null) {
        log.log(Level.SEVERE, "Cannot disconnect as mClientConnection is null");
        return false;
    }

    if(mClientConnection.isConnected()) {
        mClientConnection.disconnect();
    }

    mClientConnection = null;

    return true;
}

Normally in the past I have always gone with the second one by just return true or false. But so now I am just looking for alternative solutions.

Many thanks for any suggestions,

ant2009
  • 27,094
  • 154
  • 411
  • 609
  • 1
    possible duplicate of [Recommended way to handle problems/errors in algorithms](http://stackoverflow.com/questions/23100633/recommended-way-to-handle-problems-in-algorithms) – Vince Apr 21 '15 at 04:49
  • It depends on how it gets used. Normally, I'd expect something like `try {....} finally {disconnect();}` which means that 1. you shouldn't throw, 2. you don't need to return anything as nobody cares. Is there any place in you code where the return value gets used? +++ Sure, your use case may be different, but "make sure it's cleaned up" is the most common one. – maaartinus Apr 21 '15 at 11:55

7 Answers7

5

If you write an API that other developers will use - better take the second approach, it's easier (and cleaner) to handle from the "customer" side:

while (!disconnected()) {
    // do something else
    // sleep and try again
    // etc
}

In general - don't throw exceptions that you know how to handle gracefully!

Nir Alfasi
  • 53,191
  • 11
  • 86
  • 129
2

Neither one of them. I would throw a custom Exception, not even a RuntimeException. And if you really need that boolean for whatever reason, return trueif it was successful, even if the mClientConnection is null, the method did what it was intended to do.

Zarathustra
  • 2,853
  • 4
  • 33
  • 62
2

Assuming you are expecting to find a value in most casesthen throwing an exception is better if its null. The exception would mean that there was a problem.

If the value can be missing or present and both are valid for the application logic or if there are other people going to use you API then return a false.

Also consistency is important so keep in sync with what you do at other places

Viraj Nalawade
  • 3,137
  • 3
  • 28
  • 44
2

Generally, return a value is so much faster than throwing a exception, because exception needs reflection and take some time to build exception data.

But depending on your project design, throw a exception might be the correct choice, since calling a method without having the connection can be considered a exception case.

rkawano
  • 2,443
  • 22
  • 22
1

In this specific case I think you should do neither. If you are telling your object to disconnect and it was already disconnected (i.e. it's already null), just don't do anything.

1

If the connection is already null, your job is done and ideally you shouldn't need to flag it as a failure. You could just log a low severity message. It really depends on what you do after the disconnection and whether you use the actual outcome of the disconnection attempt

kaykay
  • 556
  • 2
  • 7
1

In addition to all the numerous answers here, I'd just like to add that throwing an exception is costly. Although the Java VM has improved so much over the years that this cost can be considered negligible, it would still be something you might want to optimize.

I'll cite another related SO question on the cost of throwing exceptions and the performance analysis created. these answers are more thorough than I can write here regarding the specifics of Java.

In short though, the throwing of exceptions is costly mainly because the machine has to unwind from the stack trace, and that information about the stack trace will always be recorded, even if it is not used, disrupting the flow of logic. A basic principle would be to put functions that might throw an exception in a try-catch block, so that the program knows how to handle itself and not crash. (users wouldn't like to see exceptions at all).

That being said, if you can choose to not throw an exception, don't do it, for performance. This is also in line with Steve McConnell's principle of exceptions in his Code Complete book, that is, only throw exceptions if they are truly exceptional.

For this particular case of the disconnect function, if you feel that having a null value is something that doesn't happen often, then throw the exception. However, if it is something that happens regularly (such as say a user not entering a value in a field, don't want an exception for that), return a false, or even handle the case within that same method, after all if the function serves to disconnect something, and it should be able to do that for all cases that happen often (already disconnected, connection dead, no reply from server etc), and only exceptions for specific cases like 2 instances of your program running, race conditions etc.

Community
  • 1
  • 1
matrixanomaly
  • 6,627
  • 2
  • 35
  • 58