CODEWITHSUNDEEP
javaconstructor
user2336315
user2336315

Reputation: 16067

Java 7 Constructor

I saw that in Java 7, they introduced the method Objects.requireNonNull(T obj, String message).

Checks that the specified object reference is not null and throws a customizedNullPointerExceptionif it is. This method is designed primarily for doing parameter validation in methods and constructors with multiple parameters.

Before starting to reformat my code, I would ask here to have some feedback about using that.

 public Foo(Bar bar, Baz baz) {
         /** Old one
         this.bar = bar;
         this.baz = baz;
         **/
         this.bar = Objects.requireNonNull(bar, "bar must not be null");
         this.baz = Objects.requireNonNull(baz, "baz must not be null");
   }

Is it a better practice to use it directly when I construct my object (I was thinking about if I create a library or other stuff for developers) ?

Or should I leave it as a "classic/old one" constructor ?

Upvotes: 23

Views: 1389

Answers (3)

Ravi K Thapliyal
Ravi K Thapliyal

Reputation: 51721

I don't feel comfortable with the fact that requireNonNull() throws a NullPointerException. In my opinion, throwing the classic IllegalArgumentException makes the intention more clear. One can use Assertions as well with the advantage that they can be selectively turned on and off.

On the top of it, if you want to expose your code as a library one should usually make an effort to support the oldest JDK possible.

EDIT: I'd like to add that pick any well known framework today and you'll find that their APIs would always throw exceptions with names so descriptive you almost instantly get to know what's wrong.

To give an example, what happens when a Spring container finds that a @Required property on a managed bean could not be injected and so has remainedNULL. You certainly don't get a NullPointerException with as descriptive an error message as possible.

You get a BeanCreationException wrapped around a BeanInitializationException giving further information about the member variable that is still null in a descriptive enough error message like: Property 'center' is required for bean 'circle'.

Upvotes: 9

Stephen C
Stephen C

Reputation: 719307

As you can see, there is disagreement whether an NPE or some other exception should be thrown1.

If you accept that an NPE is more appropriate than the alternatives, AND this is a situation where null is unequivocally incorrect, then:

  • it is better to throw the NPE early; i.e. in the constructor, and
  • it is better to throw it with a custom exception message; i.e. not a null as happens when the JVM throws NPEs.

In that context using Objects.requireNonNull is clearly good practice.

You could also use Java assertions for this, but you'd need to make a judgement as to whether it is a good or bad thing that the test could be turned off. You have to consider whether the performance benefit of turning the early check off outweighs the problems that could be caused by not checking earlier. For instance, suppose you see an NPE in production with assertions turned off ... and you can't diagnose it. Do you turn on assertions? Will the assertions change the behaviour of the code? Will it slow your system down (due to lots of other assertion checks)? Is the situation that triggered the NPE likely to recur?

(Threading-related bugs are often "once in a blue moon" things that are exceedingly difficult to reproduce. I would argue that you need the information to diagnose the problem ANY time that it occurs ... not just when you've got assertion checking enabled.)

The backwards compatibility argument is possibly relevant. However, as a general rule you don't write your code to run on old Java platforms. If you have a specific requirement to support old versions of Java ... that's different. But if that was the case, you shouldn't be doing your development against the Java 7 APIs at all ... so your compiler / IDE should flag the Objects class as compilation error.

(Restricting yourself to only using the older APIs when you don't need to will make your code quality suffer, and make it "out of date" earlier. The whole point of the new stuff is to make it easier to write reliable / maintainable applications. Deliberately not using it is ... perverse. Imagine if you restricted yourself to Java 1.1 ...)

1 - FWIW, I think NPE's are just fine. And NPE with a message is more specific than (say) IllegalArgumentException, and there's lots of precedents in the Java standard class libraries for constructors, etc that are documented as throwing NPEs. Besides, this method was clearly designed to be used this way.

Upvotes: 11

Idan Arye
Idan Arye

Reputation: 12633

If you happen to have setters for bar and baz, you should do it in the setters:

public Foo(Bar bar, Baz baz) {
    setBar(bar);
    setBaz(baz);
}

public void setBar(Bar bar) {
    this.bar = Objects.requireNonNull(bar, "bar must not be null");
}

public void setBaz(Baz baz) {
    this.baz = Objects.requireNonNull(baz, "baz must not be null");
}

Otherwise the method you showed is fine.

Regarding the question about doing it at all - if your object is completely useless when bar and baz are null, you might as well do the null check at the constructor&setters. But if your object can be used when they are null - even if that usage is just checking that they are null and setting them with a setter, assuming that is a legitimate usage of your class - then you can let your users do it.

Upvotes: 0

Related Questions