My class needs to be immutable but I need a copy constructor, and since my fields aren't initialized, the constructor gives me an error

Question

So I am currently working on a class that is supposed to be immutable.

package sysutilities;

public final class Address {

// Initializing fields/////////////////////////////////////////////////////
private final String street, city, state, zip;

//Constructor with 4 parameters - street, city, state and zip code/////////
public Address(String street, String city, String state, String zip) {

    this.street = street.trim();
    this.city = city.trim();
    this.state = state.trim();
    this.zip = zip.trim();

    if (this.street == null || this.city == null || this.state == null || 
            this.zip == null || !this.zip.matches("\\d+")) {

        throw new IllegalArgumentException("Invalid Address Argument");

    }

}

//Default Constructor//////////////////////////////////////////////////////
public Address() {

    this.street = "8223 Paint Branch Dr.";
    this.city = "College Park";
    this.state = "MD";
    this.zip = "20742";

}

//Copy Constructor/////////////////////////////////////////////////////////
public Address(Address inp) {

    Address copyc = new Address();

}

My problem arises because a copy constructor can apparently not do its job if there are final private fields in the immediate vicinity. And so my code is giving me an error there. However, if I attempt to initialize the fields as null at the start, the rest of my program gives me an error.

Is there any way I can retain the immutability of the class without have private final fields?

Answer 1

Your constructor should not create another Address object. It should initialize the fields of this address, that is being constructed by the constructor:

public Address(Address inp) {
    this.street = inp.street;
    this.city = inp.city;
    ...
}

or

public Address(Address inp) {
    this(inp.street, inp.city, ...);
}

But, as Andy Turner rightfully mentions in his comment, if a class is immutable, there is no reason to create a copy of any of its instances, since they can be reused and shared between objects and threads without any risk.

Answer 2

You don't say what error you're getting, if I run your code with the copy constructor like

public Address(Address a) {
    Address c = new Address();
}

then I get "variable street might not have been initialized", which makes sense because the instance being created by calling the Address constructor never gets any values set for its final fields. Creating a local variable in your constructor doesn't do anything to initialize the instance you're constructing. Constructors don't return anything, all you can do is set fields on the instance being initialized. Note also that the passed-in Address is going unused.

As others have said there is no good reason to make copies of an immutable object. But that's homework for you.

Your validation is faulty; if you pass in a null for street, for instance, you'll get a NullPointerException. The validation checks never get reached. (This kind of thing is why people write tests for their code.) Also the other constructors can't take advantage of the validation in the first constructor.

The constructor that takes arguments needs to check the parameters passed in before it tries to do anything with them:

public Address(String street, String city, String state, String zip) {
    if (street == null || city == null || state == null || 
            zip == null || !zip.matches("\\d+")) {
        throw new IllegalArgumentException("Invalid Address Argument");
    }
    this.street = street.trim();
    this.city = city.trim();
    this.state = state.trim();
    this.zip = zip.trim();
}

Of course the exception message is cryptic since you omit which field caused the problem. It would be better to have separate tests and throw exceptions with more specific messages.

The no-arg constructor could be chained to the other one:

public Address() {
    this("8223 Paint Branch Dr.", "College Park", "MD", "20742");
}

This way you don't have two separate paths for initializing your object. If you did have effective validation code in the other constructor, your current no-arg constructor would not be using it. But if you chain the constructors this way both paths get validated.

Changing your copy constructor to

public Address(Address inp) {
    this(inp.street, inp.city, inp.state, inp.zip);
}

would allow it to share the validation code that lives in the primary constructor. That doesn't matter a lot in this example since you're getting your inputs from a presumably valid object, but in general having constructors work together helps to avoid errors, here's an example of where an error occurred because there were different ways to initialize an object.