How to avoid constructor code redundancy in Java?
Asked Answered
U

4

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

Unstuck answered 18/6, 2013 at 14:4 Comment(2)
your problem with this() and super() have to be the [first statement in a constructor][1]. [1]: #1168845Kerby
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.Castlereagh
C
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; }
Carlson answered 18/6, 2013 at 14:6 Comment(7)
An option would be to make the fields final and let the default constructor call this(null, null);Statics
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; } ... }Scaife
@MichaelShopsin why would there be an exceptionCrissy
@Crissy 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.Scaife
@MichaelShopsin ctor chaining with final works for me, thought a constructor != a method.Crissy
@Crissy you are correct, I never tried calling other constructors only methods.Scaife
Would you mind providing a source and or some context? This is not an answer, it's only codeDryer
B
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.

Bik answered 18/6, 2013 at 14:7 Comment(4)
@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.Bik
Guys, I feel that discussing the naming of car and cdr is really off topic and a bit noisy here ;-)Statics
It is just an example. I can change it to foo and bar if you are more familiar with them.Unstuck
@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.Tayler
B
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!

Bordello answered 18/6, 2013 at 14:11 Comment(2)
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!Straitjacket
@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).Bordello
L
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;
    }
}
Lowther answered 18/6, 2013 at 14:7 Comment(0)

© 2022 - 2024 — McMap. All rights reserved.