ceving
ceving

Reputation: 23871

How to avoid constructor code redundancy in Java?

I have the following class:

class Pair
{
    String car;
    Integer cdr;

    public Pair () {}
    public Pair (String car) { this.car = car; }
    public Pair (Integer cdr) { this.cdr = cdr; }

    public Pair (String car, Integer cdr)
    {
        this(car);
        this(cdr);
    }
}

The class contains two optional values and I would like to provide all possible constructor permutations. The first version does not initialize anything, the second initializes only the first value and the third initializes only the second value.

The last constructor is the combination of the second and third one. But it is not possible to write this down, because the code fails with.

constructor.java:13: call to this must be first statement in constructor
        this(cdr);
            ^
1 error

Is it possible to write the last constructor without any code redundancy (also without calling the same setter methods)?

Upvotes: 16

Views: 5426

Answers (4)

Jon Skeet
Jon Skeet

Reputation: 1503899

Chain your constructors in the opposite direction, with the most specific being the one to set all the fields:

public Pair() {
    this(null, null); // For consistency
}

public Pair(String car) {
    this(car, null);
}

public Pair(Integer cdr) {
    this(null, cdr);
}

public Pair (String car, Integer cdr)  {
    this.car = car;
    this.cdr = cdr;
}

That way:

  • Only one place sets fields, and it sets all the fields
  • From any other constructor, you can specify (and tell when you're reading the code) the "defaulted" values for the other fields.

As an aside, I'd strongly recommend that you make the fields private (and probably final), and give them more meaningful names.

Note that this way, if you have (say) 5 parameters and one constructor with 3, one with 4 and one with 5, you might choose to chain from 3 -> 4 -> 5, or you could go straight from 3 -> 5.

Additionally, you may want to remove the single-parameter constructors entirely - it would be more readable to have static methods instead, where you can specify the meaning in the name:

public static Pair fromCar(String car) {
    return new Pair(car, null);
}

public static Pair fromCdr(Integer cdr) {
    return new Pair(null, cdr);
}

Or in my preferred naming of the meanings:

public static Pair fromFirst(String first) {
    return new Pair(first, null);
}

public static Pair fromSecond(Integer second) {
    return new Pair(null, second);
}

At this point you can make the Pair class generic, without worrying about which constructor will be called if the two type arguments are the same. Additionally, anyone reading the code can understand what will be constructed without having to check the type of the argument.

Upvotes: 19

fge
fge

Reputation: 121840

You are probably looking for the builder pattern here.

Among other things, this pattern allows you not to have a bazillion constructors covering all cases. For your situation, this could be:

@Immutable // see JSR 305
public final class Pair
{
    private final String car;
    private final integer cdr;

    private Pair(final Builder builder)
    {
        car = builder.car;
        cdr = builder.cdr;
    }

    public static Builder newBuilder()
    {
        return new Builder();
    }

    // whatever other methods in Pair, including accessors for car and cdr, then:

    @NotThreadSafe // see JSR 305
    public final class Builder
    {
        private String car;
        private int cdr;

        private Builder()
        {
        }

        public Builder withCar(final String car)
        {
            this.car = car;
            return this;
        }

        public Builder withCdr(final int cdr)
        {
            this.cdr = cdr;
            return this;
        }

        public Pair build()
        {
            return new Pair(this);
        }
    }
}

Sample usage:

final Pair newPair = Pair.newBuilder.withCar("foo").withCdr(1).build();

Advantage: Pair is now immutable!

Upvotes: 8

LaurentG
LaurentG

Reputation: 11827

class Pair
{
    String car;
    Integer cdr;

    public Pair () {}
    public Pair (String car) { 
        this(car, null)
    }
    public Pair (Integer cdr) {
        this(null, cdr);
    }

    public Pair (String car, Integer cdr) {
        this.car = car;
        this.cdr = cdr;
    }
}

Upvotes: 5

Sebastian Redl
Sebastian Redl

Reputation: 72215

As a rule, constructors with fewer arguments should call those with more.

public Pair() {}
public Pair(String car) { this(car, null); }
public Pair(Integer cdr) { this(null, cdr); }
public Pair(String car, Integer cdr) { this.car = car; this.cdr = cdr; }

Upvotes: 52

Related Questions