calling setters from a constructor

What are the pro's and con's of calling out to a mutator from a constructor (if any)

i.e.:

public MyConstructor(int x) {
  this.x = x;
}

versus:

public MyConstructor(int x) {
  setX(x);
}

public void setX(int x) {
  this.x = x;
}

Do you have a preference? (This is not homework, just looking at our coding standards doc where it says to always call out to mutators when setting instance var's in the constructor and I don't always to this)

Personally, I would set the variable directly in most cases.

Methods usually expect that the instance is fully-formed by the time they're called. In particular, calling an overridden method from a constructor is a recipe for hard-to-understand code and hard-to-spot bugs.

Having said that, I often try to make classes immutable anyway, in which case not only is there no setter, but you have to set the final variable from the constructor (or a variable initializer) anyway :)

Where properties have logic, setter logic is usually validation and sometimes change propagation to observers. I'd usually expect the constructor parameters to be checked explicitly at the start of the method, and you wouldn't want any change propagation to occur before an instance is fully created anyway.

I follow two rules about constructors to minimize problems which are why I would not use the mutator method:

Constructors (of non-final classes) should call only final or private methods. If you decide to ignore this rule and let the constructor call non-final/non-private methods, then:

those methods and any methods they may call must be careful not to assume the instance is fully initialized, and
the subclasses that override those methods (subclasses that may not even be aware that the superclass constructor is calls those methods) must not assume that the subclass constructor and superclasses' constructors have been fully executed. This problem gets worse the deeper down the inheritance hierarchy the superclass with the "evil" constructor is.

Is all that extra cognitive baggage worth it? You could allow an exception for simple mutators that only assign a value to an instance variable, since there's little benefit, even that doesn't seem worth it.

[[ @Jon Skeet mentions this in his answer: "... In particular, calling an overridden method from a constructor is a recipe for hard-to-understand code and hard-to-spot bugs." But I don't think the ramifications of this problem is stressed enough. ]]

Constructors should be cautious about leaking this before the instance is fully initialized. While the previous rule was about methods inside the class and subclasses accessing ivars, you must also be careful about (even final/private) methods passing this to other classes and utility functions before this is fully initialized. The more non-private, overridable methods that the constructor calls, the greater the risk of leaking this.

Some references about constructors calling non-final, non-private methods:

https://www.securecoding.cert.org/confluence/display/java/MET05-J.+Ensure+that+constructors+do+not+call+overridable+methods

http://www.javaworld.com/article/2074669/core-java/java-netbeans--overridable-method-call-in-constructor.html

http://www.javaspecialists.eu/archive/Issue210.html

Invoking any public, static, non-final methods within constructor it's up to you, but the best practice is never invoke such methods within constructor, because this methods can be overridden in subclasses and actually only overridden version of this methods will be invoked (If you use polymorphic behavior).

For example:

public class Foo {

    public Foo() {
        doSmth(); // If you use polymorphic behavior this method will never be invoked
    }

    public void doSmth() {
        System.out.println("doSmth in super class");
    }

    public static void main(String[] args) {
        new Bar(200);
    }
}

class Bar extends Foo {

    private int y;;

    public Bar(int y) {
        this.y = y;
    }

    @Override
    public void doSmth() { // This version will be invoked even before Barr object initialized
        System.out.println(y);
    }

}

It will print 0.

For mo details read Bruce Eckel "Thinking in Java" chapter "Polymorphism"

My preference is to set them directly in the constructor for a few reasons. Firstly something like this.x = x; is just as clear, if not more so than calling a separate method that does the same thing. Secondly the method may have potentially been overridden unless it's marked final, and calling potentially overridden methods from a constructor is a big no-no in my book. Thirdly, most methods generally assume that the object is already complete when they're executing, not half way through being constructed. Whilst this shouldn't cause any issues in this simple case, in more complex cases it can cause seriously subtle bugs that take ages to track down.

The main argument for using setters / getters everywhere is that it means you can rename the field by just changing its name in 3 places, its definition, the getter / setter methods, and all should compile and be fine. That argument is null and void these days in my opinion, since any decent modern IDE will rename all occurrences of such a field with a simple keyboard shortcut.

Unless your setter is doing something more complex than this.x = x, I would just set the variable directly. The parameter maps directly to your instance variable, and to me it's more intent-revealing.

calling setters from a constructor

Related

Recent Posts