17

So i want to do a null safe check on a value contained within a value.

So I have 3 objects contained within each other:

Person has a clothes object which has a country object which has a capital

So a person may not have clothes so a check like this throws a null pointer:

if (person.getClothes.getCountry.getCapital)

How would I make a statement like this just return false if any of the objects on the path are null?

I also don't want to do it this way. (A one-liner in Java-8 if possible.

if (person !=null) {
    if (person.getClothes != null) {
        if (person.getClothes.getCountry !=null) {
            etc....
        }
    }
}
Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
Blawless
  • 1,229
  • 2
  • 16
  • 26
  • 4
    One liner: `if (person != null && person.getClothes != null && person.getClothes.getCountry != null) { }`. – Andy Turner Sep 07 '18 at 10:30
  • 1
    What do you want the code to do? Having long chains of methods calls that access different nested objects can also be considered an anti-pattern as it violates the so called [Law of demeter](https://en.wikipedia.org/wiki/Law_of_Demeter) (a.k.a. principle of least knowledge). – Mick Mnemonic Sep 07 '18 at 10:31
  • basically, if the value exists(getCountry), take the result of that, otherwise false.... – Blawless Sep 07 '18 at 10:33
  • So could you add a method to `Person` asking his/her clothes to tell which country they are from? – Mick Mnemonic Sep 07 '18 at 10:36
  • btw you could accept the answer if it helped you... – Eugene Sep 07 '18 at 15:27

4 Answers4

23

You can chain all of those calls via Optional::map. I sort of find this easier to read than if/else, but it might be just me

Optional.ofNullable(person.getClothes())
        .map(Clothes::getCountry)
        .map(Country::getCapital)
        .ifPresent(...)
Eugene
  • 117,005
  • 15
  • 201
  • 306
  • I think it would be better to refactor the code or even use nested ifs instead of doing it like this.. – Mick Mnemonic Sep 07 '18 at 10:34
  • @MickMnemonic I don't know... I actually like it like this, but I might get affected from such much `java-8`.. – Eugene Sep 07 '18 at 10:37
  • I think a preferrable solution would be to refactor the code in an OO manner so that you could ask for the information from the `Person` directly (which would delegate the call to `Cloth` etc.), without requiring excessive lambdas. – Mick Mnemonic Sep 07 '18 at 10:41
  • 1
    @MickMnemonic could not agree more, but we have all been there, what if this is *not* an option? – Eugene Sep 07 '18 at 10:43
  • @MickMnemonic in this particular case, I don't agree. The example is perhaps a little contrived, but why would you want to expose the information about the capital city of the country of origin of a person's clothing directly through the `Person` object. – Alnitak Sep 07 '18 at 11:06
  • @Alnitak, of course it depends. But assuming that the business needs information about the origin of the clothes of a person implies that they only want to deal with `Person`s and not care about whether the person might be waering jeans or any other clothes, so to keep dependencies to a minimum, `Person` could provide a method for accessing this information to the business. The delegate calls (and corresponding dependencies) to `Cloth` etc. composites would then be encapsulated away and `Person`. – Mick Mnemonic Sep 07 '18 at 11:14
  • 2
    @MickMnemonic the solution would be to refactor the code, indeed. But to not allowing so many properties to be `null` in the first place. Is there any use in allowing the capital of a country to be `null`? I don’t think so. The association between clothes and country is above my head anyway… – Holger Sep 07 '18 at 11:37
  • @Holger ..... Its was just an abstract example to demonstrate what I was looking for. I wouldn't hurt your head trying to wrap your head around the association between clothes and country....and as for a null capital of a country, look into a country called Nauru... But thanks for your input anyhow :) – Blawless Sep 17 '18 at 12:53
  • @Blawless when you can’t find a valid real-life example, there is no point in constructing an artificial example demonstrating a problem that doesn’t exist. When it comes to a country without a de jure capital, you have to decide whether you use the de facto capital instead, or use a special value representing “no capital”. What you should *never* do, is stop processing a person (or their clothes), because their country has no capital. Think about it, it should be obvious what anomalies that will cause. So the statement still holds. Don’t let (so many) properties have a `null` value. – Holger Sep 17 '18 at 13:37
  • 1
    @Holger ... Shoot you maybe right... I never even factored in the case where the person is an exhibitionist and doesn't wear clothes at all, unless you would count socks as clothes? – Blawless Sep 17 '18 at 14:06
  • @Blawless clothes should be a kind of collection (or literally a `Collection`), so not having clothes should be represented as an empty collection, not requiring special handling in most of the cases (and never require dealing with `null`). Even when you want to check whether there’s a risk of a scandal or public nuisance, you have to iterate over them and accumulate into some “*covers required minimum for current location*” result, which works inherently for empty collections as well. – Holger Sep 17 '18 at 14:36
  • Each time you are using Optional.of - you create new Optional object. – kapellan Nov 28 '19 at 10:17
  • @kapellan true. And? – Eugene Nov 29 '19 at 05:10
  • @Eugene sometimes people overuse this kind of null check with optional in places where simple (if var != null) is match more readable and light approch – kapellan Dec 02 '19 at 21:00
  • @kapellan _more_ readable is subjective, you are aware of that right? My team has somehow different standards, at times; though a simple `!= null` is an overkill, I agree. – Eugene Dec 02 '19 at 21:12
7

These "cascade" null-checks are really paranoid and defensive programming. I'd start with a question, isn't better to make it fail fast or validate the input right before it's store into such a data structure?

Now to the question. As you have used nested the null-check, you can do the similar with Optional<T> and a method Optional::map which allows you to get a better control:

Optional.ofNullable(person.getClothes())
        .map(clothes -> clothes.getCountry())
        .map(country -> country.getCapital())
        .orElse(..)                               // or throw an exception.. or use ifPresent(...)
shmosel
  • 49,289
  • 6
  • 73
  • 138
Nikolas Charalambidis
  • 40,893
  • 16
  • 117
  • 183
4

You can achieve using single line of code

if (person != null && person.getClothes != null && person.getClothes.getCountry != null) { }

As you know there is substantial difference between = and ==.

The operators && and || are short-circuiting, meaning they will not evaluate their right-hand expression if the value of the left-hand expression is enough to determine the result

If your first expression is true then only it will check for next expression.

If first expression is false then it will not check for next expression.

So as your requirement if person is not null then only check for person.getClothes != null and so on.

halfer
  • 19,824
  • 17
  • 99
  • 186
Sagar Gangwal
  • 7,544
  • 3
  • 24
  • 38
1

As you mentioned Java -8 Here is what you would like

Objects.isNull(person) //returns true if the object is null

Objects.nonNull(person) //returns true if object is not-null

Optional.ofNullable(person.getClothes())
    .flatMap(Clothes::getCountry)
    .flatMap(Country::getCapital)
    .ifPresent(...)

By using Optional, and never working with null, you could avoid null checks altogether. Since they aren't needed, you also avoid omitting a null check leading to NPEs. Still, make sure that values returned from legacy code (Map, ...), which can be null, are wrapped asap in Optional. check here

if(Objects.nonNull(person) && Objects.nonNull(person.getClothes) &&  Objects.nonNull(person.getClothes.getCountry )){
   // do what ever u want
 }

And if you are operating with Collections and using org.apache.commons then CollectionUtils.isNotEmpty(persons) and CollectionUtils.isEmpty(persons) will works for you. Where Persons is List of person.

manikant gautam
  • 3,521
  • 1
  • 17
  • 27
  • 1
    There is no sense in using `Objects.nonNull(person)` instead of `person != null`. The method only exists to allow using `Objects::nonNull` in functional contexts, i.e. when a `Predicate` is needed. – Holger Sep 07 '18 at 11:40
  • @Holger agrred with your comment , but my answer was more close to a way in `java-8` not among which one is best . – manikant gautam Sep 07 '18 at 12:05
  • Using `… != null` still is the way to do it in Java 8 and even Java 11. Otherwise, it’s like using a smartphone to hammer a nail into the wall, “to show the 2018 way of doing it”. – Holger Sep 07 '18 at 12:08
  • @Holger it is not mine personal way , It is still java designers way. Will love to know why i can't use like that . What are the pros and cons of using and not using this. – manikant gautam Sep 07 '18 at 12:11
  • You can use either way, but the preferred way still is the same as before, for ordinary expressions. As said, the `nonNull` method has been added to allow using it in method references of the `Objects::nonNull` form. [To quote the author himself](https://stackoverflow.com/a/25452227/2711488): “*These small methods were specifically added to the APIs so that programmers could use names instead of inline lambdas*”… – Holger Sep 07 '18 at 12:18
  • @Holger make sense thnx ! – manikant gautam Sep 07 '18 at 12:25