How can I validate a copy constructor's parameter?

Question

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);
  } 
}

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

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.

Answer 1

You could introduce a private static method that returns the relevant value with a null check to throw the IllegalArgumentException (in this case the width as it's the first parameter on the same object).

For example:

public class Rectangle {

    private int width, height;

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

    public Rectangle(Rectangle rectangle) {
        this(getWidth(rectangle), rectangle.height);
    }

    private static int getWidth(Rectangle rectangle) {
        if (rectangle == null) {
            throw new IllegalArgumentException("null value");
        }
        return rectangle.width;
    }

}

Reflecting on the comment to the question above, why not NullPointerException?

This question: IllegalArgumentException or NullPointerException for a null parameter? has some good discussion points. But I'm inclined to agree with the sentiments of "Effective Java" that NullPointerException should be preferred as cited in this answer to that very question.

"Arguably, all erroneous method invocations boil down to an illegal argument or illegal state, but other exceptions are standardly used for certain kinds of illegal arguments and states. If a caller passes null in some parameter for which null values are prohibited, convention dictates that NullPointerException be thrown rather than IllegalArgumentException."

Answer 2

You can use java.lang.Objects for this, but it will throw NullPointerException (with a message your supply) instead of IllegalArgumentException:

    public class Rectangle {

    private int width, height;

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

    public Rectangle(Rectangle rectangle) {
        this(Objects.requiresNotNull(rectangle, "rectangle was null").getWidth(), getHeight(rectangle));
    }
    ...
    }

You can also make width and height final if you want to.

Answer 3

If you have this method somewhere in your codebase:

public static <T, R> R getIfNotNull(T instance, Function<T, R> extractor) {
    if (instance == null) throw new IllegalArgumentException();
    return extractor.apply(instance);
}

You could use it to throw an IllegalArgumentException from within your constructor:

public class Rectangle {

    int width, height;

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

    public Rectangle(Rectangle source) {
        this(getIfNotNull(source, s -> s.width), source.height);
    }
}

Disclaimer: all this would be needed just to throw an IllegalArgumentException, which is less idiomatic than a NullPointerException. In general, it's considered good practice to throw a NullPointerException if some non-nullable argument is null. And your original code was already doing this when attempting to dereference the argument.

Answer 4

Probably the simplest workaround is not to have your constructor call another constructor. There's no advantage to it, unless you're doing a classroom assignment where you're told you have to do it:

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

public Rectangle(Rectangle rectangle) {
    if (rectangle == null) {
        throw new IllegalArgumentException(...);
    }
    this.width = rectangle.width;
    this.height = rectangle.height;
}

Of course you now have duplicated code, but it's simple to extract that into a new private method used by both constructors.

This seems more straightforward to me than inventing a way to shoehorn a null check into the this(...), as a couple other answers are trying to do.

Answer 5

If you're open to alternatives to a copy constructor, a static copy factory would buy you more flexibility:

[...] you are usually better off providing an alternative means [to Cloneable] of object copying. A better approach to object copying is to provide a copy constructor or copy factory. A copy constructor is simply a constructor that takes a single argument whose type is the class containing the constructor [...]

(source: Josh Bloch's Effective Java, 3nd ed., item 13, p65)

public final class Rectangle {

  private final int width
  private final int height;

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

  public static Rectangle newInstance(Rectangle source) {// <-- copy factory
    if (source == null) {
      throw new IllegalArgumentException("null source");
    }
    
    return new Rectangle(source.width, source.height);
  }

}

Arguably, throwing a NullPointerException would be more idiomatic than throwing an IllegalArgumentException, though.