Multiple independent or layered constructors for TicTacToe?

At first it was like this:

public abstract class TicTacToeViewAbstract implements TicTacToeView {
    private final List<OnCellClickListener> onCellClickListeners;
    private boolean movesBlocked;
    private boolean gameFinished;
    private TicTacToeModel model;

    protected TicTacToeViewAbstract(TicTacToeModel model) {
        onCellClickListeners = new ArrayList<OnCellClickListener>();
        gameFinished = false;
        movesBlocked = false;
        plugModel(model);
    }

    protected TicTacToeViewAbstract(TicTacToeModel model, TicTacToeViewAbstract toRestore) {
        this.onCellClickListeners = toRestore.onCellClickListeners;
        this.gameFinished = toRestore.gameFinished;
        this.movesBlocked = toRestore.movesBlocked;
        this.plugModel(model);
        toRestore.unplugModel();
    }
    // other methods ...
}

Then I tried to remove duplication in both constructors:

public abstract class TicTacToeViewAbstract implements TicTacToeView {
    private final List<OnCellClickListener> onCellClickListeners;
    private boolean gameFinished;
    private boolean movesBlocked;
    private TicTacToeModel model;

    private TicTacToeViewAbstract(List<OnCellClickListener> onCellClickListeners,
                    boolean gameFinished, boolean movesBlocked, TicTacToeModel model) {
        this.onCellClickListeners = onCellClickListeners;
        this.gameFinished = gameFinished;
        this.movesBlocked = movesBlocked;
        this.plugModel(model);
    }

    protected TicTacToeViewAbstract(TicTacToeModel model) {
        this(new ArrayList<OnCellClickListener>(),
             false, false, model);
    }

    protected TicTacToeViewAbstract(TicTacToeModel model, TicTacToeViewAbstract toRestore) {
        this(toRestore.onCellClickListeners,
             toRestore.gameFinished, toRestore.movesBlocked, model);
        toRestore.unplugModel();
    }
    // ...
}

Which one looks better, and why?

Contrary to Vince, I would vote for the first version.

Guided from the principle of You Ain’t Gonna Need It, I would argue that the duplication here is very minimal and eliminating it is not really worth the cost of losing the clarity.

I would even say there’s no real duplication here at all. No program logic is duplicated. It’s only that both constructors know about these three fields and this one method. But they’re constructors – one kind’a expects that they initialize these fields. And when they instead call some other method, then I’m a bit puzzled, as this goes against my expectations of what constructors do.

In the second version there’s this new constructor with 4 arguments, and that’s too much for most methods. One needs to do spend additional mental effort to understand which arguments in here (new ArrayList<OnCellClickListener>(), false, false, model) map to which parameters.

I would vote for the second, i.e. the one with a basic constructor called by others.

But what is important is how to decide. The criteria that come to mind are:

• what if you should refactor later?
• which is the most clear when using this class?

In your case, you want to offer different constructors for different usages, with default values in some case.

There are two types of refactoring impacting a constructor:

• fields change
• usage changes

Refactoring fields

If the fields change, then you do not have to update 2 (or more) constructors by hand. You first update the lowest level constructor. You can handle the default setting of this field once. If you use an argument and forget to update other constructors, the compiler will let you know.

While if you picked the first option, you will notice you forgot to update all constructors at execution (or test if you have good tests) time.

Refactoring usage

If the usage change, you want to add a new constructor with different arguments. Then you will call another constructor, and all fields will be set since your constructors were correct at first implementation.

Again, if you picked the first option, you would have to set the fields one by one, and there are chances you would not set a field correctly, the compiler would not help you on that.

Clarity

Now let’s consider how clear it looks to a user. If you set correctly the visibility (as you did), no option is particularly preferable. Some javadoc would help though, so that the user does not need to open your file. But when reading your file, unless the javadoc is clear, the second option is more confusing, as one should read the whole code to figure what best to use, and what each constructor does, jumping to another method, …

There might be other criteria that I missed. But from a basic check, the second option seems easier to maintain and be clear enough if some documentation is added. But based on those two criteria, you might conclude differently (you might find clarity more important than later refactoring)