• Post Reply Bookmark Topic Watch Topic
  • New Topic

Null check good practice for several variables  RSS feed

 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
In my project, i have several different variables(String, Object, array...) in a class to validate for NULL. So, right now i making validate
method for each one of them. I am fed up to doing the same.

Is it ok if i used generic null method like below?



or sometime i need to throw exception as well. So is it fine?


 
Joe Harry
Ranch Hand
Posts: 10128
3
Eclipse IDE Mac PPC Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
NullPointerExceptions on your production server logs are really embarrassing!

You are right in your approach to prevent them, but at the same time you do not want to infest your code base with these null checks. I would preferably have null checks at the beginning of a tier (Facade, DAO, Business Layer)!

Having been doing some functional programming for the last couple of months, I have never encountered any null pointer problems by just using optional types!
 
Tim Cooke
Marshal
Posts: 4041
239
Clojure IntelliJ IDE Java
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I like the Guava Preconditions library for this sort of thing.

This throws a NullPointerException if the arg is null. It also returns the arg if it's not null so you can use it inline for assignments:

 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
null checks at the beginning of a tier

Yeah but still have to do for lots of them. So my question was, can i use Object type instead of any specific type?


I like the Guava Preconditions library for this sort of thing.

I can't use any 3rd party libraries
 
Tim Cooke
Marshal
Posts: 4041
239
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No 3rd party libraries? Ridiculous requirement.

If you just want to throw an exception if null then taking Object as an argument type is fine. However, if you want to mimic the Guava behaviour and return the argument if not null then you'll need generics. This is taken from the Guava source code:
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16059
88
Android IntelliJ IDE Java Scala Spring
  • Likes 2
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
No third party libraries - no problem! Because Java 8 has something similar to Google Guava's Preconditions - the class java.util.Objects.
 
Tim Cooke
Marshal
Posts: 4041
239
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
ooh nice Jesper. I wasn't aware of that addition.
 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Thanks Jesper but we still on Java 6.
 
Tim Cooke
Marshal
Posts: 4041
239
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Java 6? no 3rd party libraries? Jeepers, that's a tough gig you've got.

You know Java 6 dropped out of support over 2 years ago? If that isn't an incentive to move to a newer version, I don't know what is.
 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I asked my manager to move to latest version but he directly refused it. He said some of the client still on Java 5 and 6
so we can't move to higher versions.
 
Tim Cooke
Marshal
Posts: 4041
239
Clojure IntelliJ IDE Java
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I'd be inclined to dig a little deeper into what he means by that, as I suspect he thinks there's a compatibility issue where there potentially is none.
 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yeah may be.. Actually i know one of the client which uses some database and performing some work on it. As per them,
when they use higher versions then they unable to do work (God knows the reason) and also they are also using our
product. Due to that they stick to older version so do us.
 
Campbell Ritchie
Marshal
Posts: 56534
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
You can write your own Objects class, as many people did before Java7 (when the class Jesper mentions became available). That should take you a good ten minutes, including the canEqual and hashCodes methods.

There have been nasty security problems found with some older Java® installations so you should use the most recent. Old versions of Java5/6 are no longer safe to use. You can of course program with Java5 compatibility.
 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yeah Thaks.. also i can do like Tim mentioned in early posts.

 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Tim Cooke wrote:You know Java 6 dropped out of support over 2 years ago? If that isn't an incentive to move to a newer version, I don't know what is.

I know from first-hand experience that many IT departments are so backwards in their engineering practices that it wouldn't surprise me at all if it took another 2 years and/or a serious system breach for them to start migrating off of Java 6. Fear and inertia eat incentives to upgrade for breakfast.
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Tushar,

Objects in the inner layers of your application should not have to check for nulls. The responsibility for checking for nulls lies in the layers closer to the where input is happening so creating your own version of the util.Objects class is a good idea, then use it at the points where data is entering your system, somewhere near the servlet layer for web apps or close to the service handlers if you're using web services or as you are reading in data from the file system or whatever. You get the idea, hopefully.

In the inner layers, just let the NPEs happen. You should see these as you are unit testing and that should be your motivation to add null checks in the appropriate outer layer.

The approach of checking for nulls everywhere in your application code is like putting swipe card entry locks on all the doors inside your house, including your pantry and bathrooms, which would just be ridiculous. You validate data at the point of entry into your system. Once it's in, it should move freely about.

If you were writing code that is to be distributed as part of a general-use library, however, you may want to throw an IllegalArgumentException instead of just a plain old NPE.
 
J. Kevin Robbins
Bartender
Posts: 1801
28
Chrome Eclipse IDE Firefox Browser jQuery Linux MySQL Database Netbeans IDE
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Junilu Lacar wrote:You validate data at the point of entry into your system. Once it's in, it should move freely about.

That is probably the most useful and succinct bit of information I've read all month. That should be your signature line.

Thanks for that. I won't forget it.
 
Mike. J. Thompson
Bartender
Posts: 689
17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I agree with Junilu completely. Checking for null and then throwing a null pointer exception is redundant as that will be thrown anyway.

Still, its still on my companies coding standards that all parameters should be checked. It's an easy thing for my manager to comment about in code reviews. Thankfully I have sufficient authority that I can overrule him.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Joe Harry wrote:NullPointerExceptions on your production server logs are really embarrassing!

True, but probably better than the alternative - having a production program silently do the wrong thing.

@Tushar: NPE's are one of the most pernicious and nasty kind of errors to track down because they are almost always an effect, not a cause - especially if they are thrown by the JVM.

For this reason, my general rules of thumb (and not everyone will agree with them) are:
1. Don't let the JVM throw an NPE. Ever. You've already been given some good advice on how to avoid this.
2. Don't write methods that return null unless you document them copiously in 42-point bold type.
3. Avoid throwing NPE yourself. My usual choice is IllegalArgumentException or IllegalStateException.
4. Fail fast, and fail LOUD.

HIH

Winston
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:
2. Don't write methods that return null unless you document them copiously in 42-point bold type.
3. Avoid throwing NPE yourself. My usual choice is IllegalArgumentException or IllegalStateException.
4. Fail fast, and fail LOUD.

And fail small - test and code incrementally. If you use automated tests, then you get all three: fast, small, and LOUD.

Great points, Winston.
 
Mike. J. Thompson
Bartender
Posts: 689
17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote:1. Don't let the JVM throw an NPE. Ever. You've already been given some good advice on how to avoid this.
...
3. Avoid throwing NPE yourself. My usual choice is IllegalArgumentException or IllegalStateException.


I certainly agree that throwing IllegalArgumentException or IllegalStateException is better than NullPointerException, but fundamentally I don't think code is improved by not letting the JVM throw a NullPointerException:

Firstly they are both RuntimeExceptions, so the end result will be the same. Either your code is catching them (which would be a poor design decision), or your program will exit and hopefully a statck trace will be recorded somewhere. The message with a JVM thrown exception is a bit rubbish (I think it just says 'null' or something to that effect), but in general the code throwing the IllegalArgumentException wouldn't have any extra information (especially if you're checking parameters in a method). The stack trace is where the real information is (so you can track exactly where the null might have come from). Of course if your production code is not compiled with debug settings then the stack trace information disappears, but I think it's generally a mistake to not compile with debug enabled as you lose so much information.

Also it adds complexity to the code. You either have to add in tests to check that the IllegalArgumentException is thrown, or have reduced code coverage statistics.

I definitely agree with your other points though, especially not returning null.
 
Junilu Lacar
Sheriff
Posts: 11477
180
Android Debian Eclipse IDE IntelliJ IDE Java Linux Mac Spring Ubuntu
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Mike. J. Thompson wrote:You either have to add in tests to check that the IllegalArgumentException is thrown, or have reduced code coverage statistics.

RTEs are best caught -- and by that I mean "discovered," not caught with a try-catch block -- during development, while unit or integration testing. The IAE is more useful when it indicates that some kind of API contract has been violated. An NPE is more general and could mean that any number of things went wrong.
 
Mike. J. Thompson
Bartender
Posts: 689
17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yes, I agree with that (I hadn't considered the more general meaning of 'catch'). NPE's are defects that should be removed.
 
Winston Gutkowski
Bartender
Posts: 10575
66
Eclipse IDE Hibernate Ubuntu
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Mike. J. Thompson wrote:I certainly agree that throwing IllegalArgumentException or IllegalStateException is better than NullPointerException, but fundamentally I don't think code is improved by not letting the JVM throw a NullPointerException:

I think it really depends on when it happens. The worst kind of NPE is one that occurs many places removed from where it was caused - hence: fail FAST.

The other problem, as you said, with NPEs thrown by the JVM is that the message (if any) is almost always useless, so I generally try to avoid them if I can. And point 3 is simply to ensure that you know that any NPE you get will have been thrown by the JVM.

Now, in a situation where an argument is immediately used - ie, you call one of its methods - you could certainly argue that checking for nulls yourself is overkill, because the error occurs straight away and the stacktrace will be crystal clear; but I'd still say that you should document it religiously.

If I call a 3rd party method that doesn't document when (or that) it throws an NPE, I will always check the arguments I pass myself - possibly redundantly - so good documentation is really important.

Fun discussion though.

Winston
 
Mike. J. Thompson
Bartender
Posts: 689
17
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
It definitely is fun. Much of what I learn comes from discussions like this
 
Tushar Goel
Ranch Hand
Posts: 934
4
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Yup discussion like this is always simple but more fruitful.
 
Campbell Ritchie
Marshal
Posts: 56534
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote: . . .
@Tushar: NPE's are one of the most pernicious and nasty kind of errors to track down because they are almost always an effect, not a cause - especially if they are thrown by the JVM.
. . .
I would throw an NPE from the methods myself, but I can see I am in a minority there. We have had this discussion before and I think I concluded who cares what sort of Exception is thrown, but it must be thrown early.

You are a little imprecise there; the NPE is not the error. It is letting nulls wander around that is the mistake. As you said, the Exception is the effect not the cause, and the real cause where the null found its way into the program can be hundreds of line or hundreds of pages away, and therefore difficult to find. I agree with the advice to make sure nulls don't get into the program in the first place. If you keep the nulls out then there can't be any to cause problems.
 
Campbell Ritchie
Marshal
Posts: 56534
172
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Winston Gutkowski wrote: . . . If I call a 3rd party method that doesn't document when (or that) it throws an NPE, I will always check the arguments I pass myself - possibly redundantly - so good documentation is really important. . . .
Fail fast and fail early?

Also declare fast and declare early. It took me a long time to realise that methods like this throw an NPE if you pass null, but the link doesn't tell you that. This method introduced in Java8 does however say, “throws NullPointerException if …” I find that confusing, and the fact that you scroll up and then read this
The API Documentation wrote: Unless otherwise noted, passing a null argument to a constructor or method in this class will cause a NullPointerException to be thrown.
…does not in my opinion reduce the confusion much.

I would prefer every method in that class to have this at its beginning:-Declare fast and declare early then everybody reading the documentation will know about such Exceptions.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!