0

In code reviews I stumbled over this java-pattern to avoid NPE's:

value = (null != someObject) ? someObject.someOtherMethod() : null

I doubt that this is "good style". I rather think that it delegates the NPE (BTW this sounds similar to mapping over a Scala-Option-value or a Haskell-Maybe... )

Is it good style or not? If not, how to improve ?

Edit: Supposing it scatters your code, the code will get unreadable: Is this already encapsulated in some libraries / API ? (apache-commons, google-coomons or similar !?)

Bastl
  • 2,926
  • 5
  • 27
  • 48
  • Style value judgement questions are off-topic for StackOverflow. You might try http://programmers.stackexchange.com but I don't know if it's their sort of thing either. – T.J. Crowder Oct 30 '12 at 09:36
  • `how to improve` - Well, as far as just this line of code is concerned, there is nothing to improve, until we know where you would be using your `value`? – Rohit Jain Oct 30 '12 at 09:37
  • 1
    @T.J.Crowder. May be [codereview.se] is the place. – Rohit Jain Oct 30 '12 at 09:37
  • 3
    This is a perfectly valid piece of code and exactly what one needs in certain contexts. It defers an NPE only if it is used in a wrong place. – Marko Topolnik Oct 30 '12 at 09:38
  • removed the styling tag. but I still wonder, if this is in any libraries ... – Bastl Oct 30 '12 at 09:49
  • In scala, you'd use `val value = Option(someObject).map(_.someOtherMethod)` and that's all really basic behavior. – Dylan Oct 30 '12 at 11:55

4 Answers4

3

This pattern is neither good or not good. You'd need to evaluate it in context.

The key question is why someObject can have null values, and whether it is meaningful ... or a bug.

  • On the one hand, null might have a specific meaning (e.g. something like "the value is unspecified"), and the code might be dealing with that in a legitimate way.

  • On the other hand, null might be a symptom of a bug (e.g. someone forgot to initialize something). In that case, the code is spreading the damage and / or making the source of the bug harder to find ... and therefore bad.

As a general observation, a lot of code / coders take the view that the best way to avoid unexpected NPEs is to liberally scatter stuff like this throughout the code-base as a preventative measure. In fact, this is exactly the wrong thing to do. The right approach is to let the NPE happen (or better still, force it to happen -- see @vacuum's Answer), figure out where the unexpected null is coming from ... and remove the source. And (of course!) you need to test extensively so that the unexpected nulls get picked up before you go into production.

IMO, it is best practice to treat a null as an error unless it has a well defined and documented meaning.

Stephen C
  • 698,415
  • 94
  • 811
  • 1,216
2

Maybe Guava will help you.

Also see similar qustion.

Community
  • 1
  • 1
vacuum
  • 2,273
  • 3
  • 20
  • 32
1

If the null can eventually be handled gracefully but cannot be handled there, then it's a perfectly reasonable pattern.

If the null assuredly can't be handled gracefully, then it's counterproductive. Better to fail fast.

Rex Kerr
  • 166,841
  • 26
  • 322
  • 407
0

This idiom is impossible to encapsulate in an API. It would need first-class language support (or the language would need to support macros), especially if you want to generalize to arbitrarily many steps in the return-derefence-return chain.

I like to avoid littering code with these kinds of checks so in some projects I found myself using a piece of reflection code that accepts a string, which it interprets as a chain of method calls, and does the right thing. Type-unsafe, of course, but many times that is not a major concern.

Marko Topolnik
  • 195,646
  • 29
  • 319
  • 436