Java null arguments when chaining Constructors
Asked Answered
A

5

9

Let's say I have a class with multiple constructors, one of which is a copy-constructor (to copy an object):

public class Rectangle {

    int width, height;

    public Rectangle(int width, int height) {
        this.width = width;
        this.height = height;
    }

    public Rectangle(Rectangle source) {
        this(source.width, source.height);
    }
}

Is there any way I can make check if source is null in the copy-constructor and throw an IllegalArgumentException if it is? Because the other constructor call has to be the first statement in my constructor.

And answered 29/7, 2016 at 9:42 Comment(4)
Why does the other constructor call have to be the first statement in the copy constructor?Cameroun
Because that's what Java wants.And
@Janno: Because that's how Java works. You can't use this(...) after another statement.Hardpressed
I would leave it as NPE when a null is passed.Moue
D
14

You can do this:

public Rectangle(Rectangle source) {
     this(checkNotNull(source, "Source cannot be null").width, source.height);
}

private static <T> T checkNotNull(T t, String msg) {
    if (t == null) throw new IllegalArgumentException(msg);
    return t;
}

I also agree with Jon Skeet that a NullPointerException is not a bad bevahiour in this case. The only thing is that in long lines when you get an NPE it can be a bit hard to identify which object is null, which is why a more specific message can be useful.

You can also not reinvent the wheel and use standard java.util.Objects methods if you don't bother throwing a NullPointerException instead:

public Rectangle(Rectangle source) {
     this(Objects.requireNonNull(source, "Source cannot be null").width, source.height);
}

if your error message is expensive to build, you can provide a Supplier<String> instead, to pay the cost of the construction of the message only when it's actually needed:

 public Rectangle(Rectangle source) {
     this(Objects.requireNonNull(source, () -> explainError(source)).width, source.height);
}
Danas answered 29/7, 2016 at 9:46 Comment(1)
checkNotNull method should return a value of type 'T'.Jefferson
H
8

Yes, you can use a helper method which will throw the exception if necessary, and return the original value otherwise... you can call that within your constructor invocation, as you're allow method calls as part of argument evaluation.

// In a helper class
public static <T> T checkNotNull(T value) {
    if (value == null) {
        throw new IllegalArgumentException();
    }
    return value;
}

Then use it as:

public Rectangle(Rectangle source) {
    this(Helper.checkNotNull(source).width, source.height);
}

However... I believe that NullPointerException is the recommended exception to throw here anyway (in Effective Java 2nd edition, for example), which your existing code will throw already. So you quite possibly don't want to make any change to your existing code.

If you want a helper method for checks like this but are happy for it to throw NullPointerException, I'd recommend using Guava and its Preconditions class, which has this and a lot of other helpful checking methods.

Also note that Java 1.7 introduced java.util.Objects which has requireNonNull, so you don't even need a third party library.

Hardpressed answered 29/7, 2016 at 9:46 Comment(6)
Is NullPointerException relly the way to go? I thought to have seen IllegalArgumentException to be thrown is such a situation before.And
@And java.util.Objects.requireNonNull throws a NullPointerException, so yeah that's ok. Check out my answer for seeing a few examples ( after I edit it)Danas
@And NPE means that you used a null value when you should have used a reference to an object.Moue
@kalsowerus: NullPointerException is recommended by Josh Bloch, and is what Google's style guide uses. That's my main source of truth on these things :)Hardpressed
@Dici: Ooh, I hadn't seen Objects.requireNonNull. Will edit that in here.Hardpressed
@And IMHO : both extend RuntimeException so it will not change the behavior but using IllegalArgumentException changes the semantic which is not good for code readability.Gualtiero
N
3

One text-book trick is move the initialization out of the constructor to a method. Then, you can have whatever code you want before it:

public class Rectangle {

    int width, height;

    public Rectangle(int width, int height) {
        init(width, height);
    }

    public Rectangle(Rectangle source) {
        if (source == null) {
            throw new IllegalArgumentException("source can't be null!");
        }
        init(source.width, source.height);
    }

    private void init(int width, int height) {
        this.width = width;
        this.height = height;
    }
}
Neptune answered 29/7, 2016 at 9:47 Comment(4)
Note that that means width and height can't be final, which is often a very significant downside.Hardpressed
ALso, private final is redundant since private methods cannot be seen from the subclassesDanas
@Danas yup, that's true, removed the final.Neptune
@JonSkeet interesting point about this technique prohibiting the members from being final. Thanks!Neptune
D
2

If you really do want to throw an IllegalArgumentException, I think the cleanest solution is to use a static method instead of a constructor:

public static Rectangle from(Rectangle source) {
    if (source == null) {
        throw new IllegalArgumentException("source can't be null!");
    }
    return new Rectangle(source.width, source.height);
}

Or you could just add a copy method:

public Rectangle copy() {
    return new Rectangle(this.width, this.height);
}

I'd prefer the latter, since it removes the need to concern yourself with the Rectangle possibly being null. Note that this will cause a NPE if you use this with a null Object, which is possibly a further indication that an NPE is fine.

Dowling answered 29/7, 2016 at 10:12 Comment(1)
Note that the copy-approach may be problematic in case of inheritance - all subclasses would have to override this in order to avoid unexpected behavior.Bifrost
H
1

You can do like this

int width, height;

public Rectangle(int width, int height) {
    this.width = width;
    this.height = height;
}

public Rectangle(Rectangle source) {
   if(source != null) {
      width = source.width;
      height = source.height;
   }
}
Hasen answered 29/7, 2016 at 9:55 Comment(3)
He wanted to avoid code duplication by calling into the base constructor and throw a validation exception. This code meets neither of these two requirementsDanas
Ah ok,thanks a lot. I'm sorry but it's my first answer on here.Hasen
No worries :). Good journey on Stack OverflowDanas

© 2022 - 2024 — McMap. All rights reserved.