Need generic way for null check

Question

I need a generic way for null check. I came up with a requirement. This is my setter method for a class:

placeHolderValues.setCurrentAddressCountry(communicationSO.getApplicationDetailSO().getApplicantDetails().get(0).getAddressDetail().get(0).getCountry());

I want to set values in placeHolderValues.setCurrentAddressCountry() only when value is present. If any of the getApplicationDetailSO() or getApplicantDetails() or getAddressDetail() or getCountry() is NULL then I have to set NULL in placeHolderValues.setCurrentAddressCountry(). I have almost 30 such types of setter.

I need a common method to achieve the same.

I wonder if you are using java 8? Then try using Optional, it's a great solution for nullpointerexception and you also be able to provide default value if it's null. — Huy Nguyen, Oct 15 '18 at 11:48
So the problem is that your getter chain could result in NPEs and you don't want to check each call's result? In that case Optional would indeed be one option (no pun intendend) as huy already suggested. — Thomas, Oct 15 '18 at 11:48
I am not sure about why a simple null-check in the callee won't suffice, however I will note that your call is a god-call, which is a type of code smell and 30 different setters, that all do more or less the same, may tell you, that the code architecture is bad. — Koenigsberg, Oct 15 '18 at 11:49

score 4 · Answer 1 · answered Oct 15 '18 at 11:50

4

A serious non-answer here: don't do it that way.

Using getA().getB().getC().getD()...getTheThingYouWant() is really bad practice. It is a clear violation of the Law Of Demeter.

So besides the problem you run into (about checking null), such constructs are really actually evil. You see, the class using this code must know that an A has a B. And that B has a C. And so on. This directly couples this class with all classes A, B, C, ... and so on.

The real answer is to avoid such changing. If at all, you do getA().getMeWhatINeed().

The only other alternative: validate your input data completely (in an upfront step), before going down such call chains.

answered Oct 15 '18 at 11:50

GhostCat

137,827
25
176
248

1

Agree. As mentioned in the comments, this is a common type of code-smell, see god-line: https://en.wikipedia.org/wiki/Code_smell – Koenigsberg Oct 15 '18 at 11:52
@Mär You are very welcome! – GhostCat Oct 15 '18 at 11:53
@Mär Agreed that this is sort of a code-smell, but probably not a "god-line" type. After all, the problem persists even if you split the line up into several lines, as in [this question](https://stackoverflow.com/q/10391406/1639625). – tobias_k Oct 15 '18 at 12:10
@tobias_k Afaik this is a god-line type, simply due to length and readability. – Koenigsberg Oct 15 '18 at 12:20

tobias_k · Answer 2 · 2018-10-15T12:12:45.670

2

You could use Optional.ofNullable and repeatedly map the value into another, unless you encouter null on the way, and then get the final value orElse null.

var value = Optional.ofNullable(communicationSO)
        .map(x -> x.getApplicationDetailSO())
        .map(x -> x.getApplicantDetails())
        .map(x -> x.get(0))
        .map(x -> x.getAddressDetail())
        .map(x -> x.get(0))
        .map(x -> x.getCountry())
        .orElse(null);
placeHolderValues.setCurrentAddressCountry(value);

This still is not very pretty, but at least you do not have to repeat the different method calls. Some of those lambdas (->) could probably also be replaced with proper Method references. (::) Still, it's probably better than just wrapping the line into a try block and catching the NullPointerExceptoin.

edited Oct 15 '18 at 12:12

answered Oct 15 '18 at 12:02

tobias_k

81,265
12
120
179

That is neat. And you could actually drop the lambda, and use method references, right? – GhostCat Oct 15 '18 at 12:12
@GhostCat At least in some cases, but not for e.g. the `get(0)`. – tobias_k Oct 15 '18 at 12:13
i have almost 3o setters. i need a generic method for this – Ashwani ojha Oct 15 '18 at 12:14
Makes sense. Obviously, the current approach is nicely readable. – GhostCat Oct 15 '18 at 12:14
1

@Ashwaniojha No, honestly. You *need* to understand that you are trying to fix a symptom. Meaning: you are in need for workarounds because your underlying design is far from ideal ... – GhostCat Oct 15 '18 at 12:15
@Ashwaniojha Well, I guess if by "generic" you mean "without rewriting all my code", then probably the easiest way would be to `try/catch` the NullPointerException. But that's one code-smell more, then (also catching any NPE that might occur within those methods, possibly hiding severe bugs). – tobias_k Oct 15 '18 at 12:15
@Ashwaniojha In fact, you could probably semi-automatically translate your exisitng code by replacing `.(` with `).map(x->x.(` in those parts of your code. – tobias_k Oct 15 '18 at 12:17

Need generic way for null check

2 Answers2