1

I have a method that could return NullPointer and in this case, a default 0.0 value must be used.

I'm wondering if there are differences between

double a=0.0;
try{
  a=somemethod();
catch(NullPointerException exc){
}
return a;

and

double a;
try{
  a=somemethod();
catch(NullPointerException exc){
  a=0.0;
}
return a;

If yes what is the best approach?

NOTE:

somemethod() is just a sample, really It is a method of a library that I cannot edit or fix to avoid NullPointer at the source so I must use catch block.

Maddy
  • 4,525
  • 4
  • 33
  • 53
AndreaF
  • 11,975
  • 27
  • 102
  • 168
  • Caching NullPointerException is a code smell. – Suresh Atta Dec 02 '17 at 10:52
  • 1
    Does `mymethod` return a `double` primitive or a `Double` object? – fgb Dec 02 '17 at 10:58
  • 1
    I suggest that you are returning `Double` from `mymethod` and that can be `null`. You are then assigning that to a `double` which causes an NPE. Change `a` to a `Double` and check for `null`. – Boris the Spider Dec 02 '17 at 11:00
  • A method that throws an NPE is in error - catching an NPE is never the answer. The answer is to _not call_ the method in circumstances where it will throw the NPE. It is **never** okay to catch an NPE. – Boris the Spider Dec 02 '17 at 11:04
  • 1
    @BoristheSpider The method has a design problem that basically when the value to return is null returns a NullPointer when It should return 0.0 that are values that are still useful to my computation, and It's a third part library that I cannot edit, catch the NullPointer is the only way to avoid the crash of the application and see the software working correctly. So, I'm sorry but your affirmation is a superficial generalization that in this case is already proven wrong. – AndreaF Dec 02 '17 at 12:22

4 Answers4

1

First: the use-case looks flawed: you should not need to catch an NPE and then use a default value. Maybe the method should throw some other Exception or return the default value itself.

Apart from that I'd use the 2nd approach because then the compiler can warn you if you forget to set the value in one of your code-branches and you don't have an empty catch block.

Consider this:

double a=0.0;
try{
  // for some reason we forget to assign a
  // the function would return 0 and you don't notice this mistake
  mymethod(); 
} catch(NullPointerException exc){
  // also an empty catch block is kind of smelly
}
return a;

vs. this:

double a;
try{
  mymethod();
  // now the compiler knows that you did not assign a value to a
} catch(NullPointerException exc){
  a=0.0;
}
return a;

In the later version the compiler can show you this message Variable 'a' might not have been initialized. E.g. screenshot from IntelliJ IDEA: enter image description here

TmTron
  • 17,012
  • 10
  • 94
  • 142
  • Any solution that suggests `catch(NullPointerException exc)` deserves a downvote. – Boris the Spider Dec 02 '17 at 10:58
  • 1
    @BoristheSpider I don't suggest that, I just answer the question: read the first paragraph! – TmTron Dec 02 '17 at 10:59
  • Walking into an XY problem is no excuse. – Boris the Spider Dec 02 '17 at 11:00
  • @BoristheSpider: not sure what you mean. The OP has mentioned that `myMethod()` is part of a 3rd party lib, so there is no way around catching the NPE, right? – TmTron Dec 02 '17 at 11:07
  • @TmTron the second method isn't a problem for the compiler otherwise I would not have asked this question. – AndreaF Dec 02 '17 at 12:17
  • @AndreaF yeah I know that both work. But you asked which of your 2 code examples is "the best approach". And IMHO, the 2nd one is slightly better for the reasons explained in my answer. – TmTron Dec 02 '17 at 12:42
0

I have a method that could return null value and in this case a default 0.0 value must be used.

Catching NullPointerException is a code smell. And you don't need it here anyway.

And coming to the point, you said, it returns a null, so it wont throw NPE.

What you have to do is

a=mymethod();
if(a ==null){
 a=0.0;
} 
Suresh Atta
  • 120,458
  • 37
  • 198
  • 307
  • unfortunately this crash at the line `a=mymethod();` – AndreaF Dec 02 '17 at 10:56
  • @AndreaF So the problem area is `mymethod` ? – Suresh Atta Dec 02 '17 at 10:57
  • 1
    @suresh atta. Yes but mymethod is part of a third part library that I cannot edit – AndreaF Dec 02 '17 at 10:59
  • @downvoter, would like to see your thoughts or answers ;) – Suresh Atta Dec 02 '17 at 11:03
  • @AndreaF So from stacktrace and source code of the lib, is it hard to trace what causing the nullpointer ? – Suresh Atta Dec 02 '17 at 11:11
  • this answer does not solve the problem, because `mymethod()` from the 3rd party lib throws the NPE - so the null check will never be invoked. – TmTron Dec 02 '17 at 11:17
  • @suresh atta I know what cause the nullpointer but unfortunately I cannot fix to avoid it because It's a third part library that cannot be fixed as I have said. Why all this hate against catch? – AndreaF Dec 02 '17 at 12:11
  • @AndreaF the problem is not exception handling with `catch` in general. We should just not have to catch NPEs: see this [nice SO answer](https://stackoverflow.com/a/18266490/6287240) – TmTron Dec 02 '17 at 12:47
  • @TmTron I know that "usually" you should avoid NPE at source but since as I have said It cannot be done in this case you have to handle these with a catch unless you prefer to rewrite a whole software infrastructure to avoid to use a library that hasn't considered that the case 0 should return a null value and not a null pointer. General rules aren't always applicable and must be contextualized to specific situations. – AndreaF Dec 02 '17 at 13:17
  • @AndreaF yeah. I know and I fully agree. You should try to explain this To BorisTheSpider :) – TmTron Dec 02 '17 at 13:28
0

this depends on your policy whether you want to throw exception based on exception of mymethod() or not.

if not and mymethod() could return null, you can become aware of this in catching block and set 0.0 value (without checking a==null in case of Double usage).

 double a;

   try{
       a =mymethod();
   catch(NullPointerException exc){
       /*log warning or error*/
       a = 0.0;
   }

   return a;
Amir Azizkhani
  • 1,662
  • 17
  • 30
-1

In Java 8, you can use

Optional.ofNullable(mymethod())
          .orElse(0.0);

Now that this is less efficient than the explicit null check, but I find it quite readable.

daniu
  • 14,137
  • 4
  • 32
  • 53
  • This is not what `Optional` should be used for. This is just a glorified `null` check. `Optional` is designed to be the return value of a method that can return "absent". Wrapping then unwrapping immediately is a code smell. – Boris the Spider Dec 02 '17 at 11:02
  • 1
    This will not catch the NPE that is thrown by `mymethod()`. – NickL Dec 02 '17 at 11:03