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.

I rarely do it, and I've never had an issue because of that. It can also have unintended consequences, especially if your setters are overridden in a subclass, or if your setters fire off additional method calls which might not be appropriate during initialization.

There is a single reason why our coding standards require the use of accessors (which may also be private). If you quickly want to find out what code is changing a field, you simply bring up the call-hierarchy for the setter in Eclipse!

This is a huge time-saver in a codebase that has reached 2 million lines of code and also eases refactoring.

It depends on how you treat setters. If you believe that the class can be derived from, you might allow such a call out so that you can override the behavior elsewhere. Otherwise, you can set it as it.

Of course, Allowing setters explicitly be called would also provide ways to inject additional behaviors during the setting (such as applying security if need be).

I dont mind either and I don't have strong pref [and dont adhere to some strict standards], just use your judgment. But if you go w/ setters anyone who overrides the class must be aware the object may not be fully created. That being said make sure no extra code in the setters can publish this reference.

Setters can be useful for standard range checks and so on, thus no extra code to verify the input is needed. Again using setters is slightly dirtier approach.

Using the field has obvious benefits (like making sure whoever subclass will not be able to maliciously alter the behavior) and it's usually preferred, esp. in libraries.

Actually c-tor is one the 3 possible way to create an object (serialization + clone are the other 2), some transient state might need to be handled outside the c-tor, so you still have what to ponder about fields vs setters. (Edit, there is another half way: Unsafe but that's what serialization uses)