16

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)?

ceving
  • 21,900
  • 13
  • 104
  • 178
  • 2
    your problem with this() and super() have to be the [first statement in a constructor][1]. [1]: http://stackoverflow.com/questions/1168345/why-does-this-and-super-have-to-be-the-first-statement-in-a-constructor – bNd Jun 18 '13 at 14:15
  • You call the this(...) constructors twice, you can only have a constructor call one other constructor, and the chained constructor invocation has to be the first statement in the constructor. – RudolphEst Jun 18 '13 at 14:41

4 Answers4

52

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; }
Sebastian Redl
  • 69,373
  • 8
  • 123
  • 157
  • 12
    An option would be to make the fields `final` and let the default constructor call `this(null, null);` – Lukas Eder Jun 18 '13 at 14:08
  • The exception to the rule of constructors calling their parents is final variables. For a version of Pair with final variables the code looks like: `class pair { private final String car; private final Integer cdr; public Pair() { this.car = null; this.cdr = null; } public Pair(String car) { this.car = car; this.cdr = null; } ... }` – Michael Shopsin Jun 18 '13 at 15:51
  • 3
    @MichaelShopsin why would there be an exception – josefx Jun 22 '13 at 14:21
  • @josefx final class variables must be set in the constructor or when they are declared. You cannot call another method from the constructor to set a final class variable. – Michael Shopsin Jun 24 '13 at 18:32
  • 2
    @MichaelShopsin ctor chaining with final works for me, thought a constructor != a method. – josefx Jun 24 '13 at 19:44
  • 1
    @josefx you are correct, I never tried calling other constructors only methods. – Michael Shopsin Jun 25 '13 at 13:38
  • Would you mind providing a source and or some context? This is not an answer, it's only code – inetphantom Jan 23 '20 at 08:19
19

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.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • @ceving: That seems to me to be a pretty obscure meaning within the context of Java... particularly as they're meant to be *operators* rather than *values*. If these are just meant to be "first" and "second" values, then those names would be much more meaningful to most people. – Jon Skeet Jun 18 '13 at 14:15
  • 3
    Guys, I feel that discussing the naming of `car` and `cdr` is really off topic and a bit noisy here ;-) – Lukas Eder Jun 18 '13 at 14:26
  • 1
    It is just an example. I can change it to `foo` and `bar` if you are more familiar with them. – ceving Jun 18 '13 at 14:27
  • 1
    @ceving: as Jon says, it would be better to rename them to `first` and `second`. what you have here is a 2-tuple, not a cons cell. – Nathan Hughes Jun 18 '13 at 14:36
8

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!

fge
  • 119,121
  • 33
  • 254
  • 329
  • nice and comprehensive example for a builder pattern, but maybe a little overkill in this case. (also, immutability is not automatically an advantage i think). But still +1 for a nice alternative! – Dennis Jun 18 '13 at 14:15
  • 2
    @Chips_100 I have become so used to do like this that, on the other hand, I may miss some important points... For instance, DI frameworks (Dependecy Injection). – fge Jun 18 '13 at 14:25
5
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;
    }
}
LaurentG
  • 11,128
  • 9
  • 51
  • 66