Why it is not a good idea to call Set method from constructor?

Question

Is it true only in Inheritance or most of the cases ?

public class MyClass {
   public int id;

     public MyClass() {
         // Some stuff 
         setId(5);
     }

     public setId(int id) {
         this.id = id;
     }
}

Answer 1

It is very true.

Because setters are always public methods. And if you class is not final then there is issue of alien method call. Which is not thread safe i.e. it is known as escaping of this reference. So from a constructor if you are calling a method it should be final or private. Else safe initialization of object will not happen which causes many bugs in real systems.

Apart from the above we should never call public method from the constructor because if the class is intended for inheritance than Constructors must not invoke overridable methods, directly or indirectly.

If you violate this rule, program failure will result. The superclass constructor runs before the subclass constructor, so the overriding method in the subclass will be invoked before the subclass constructor has run. If the overriding method depends on any initialization performed by the subclass constructor, the method will not behave as expected.

source.

Answer 2

You should not call an overridable method from a constructor.

If you call a method that can be overridden by a subclass in a costructor, than the subclass might access variables of the superclass that have not been initialized yet.

For example the following code looks good so far

class MyClass {
    public int id;
    protected String someStr;

    public MyClass() {
        SetId(5);
        someStr = "test";
    }

    public void SetId(int Id) {
        id = Id;
    }
}

If you now subclass MyClass and override the SetId method you might access the superclass's someStr variable which has not been initialized yet and thus will cause a NullPointerException in this case.

class MySubClass extends MyClass {

    public void SetId(int Id) {
       id = Id;
       someStr.toString(); // will cause NullPointerException
    }
}

The cause of the NPE might be hard to see if there is a bigger inheritence hierarchy.