How to fix "Constructor Calls Overridable Method"

Question

I have the following setup which is giving me a message stating that "Constructor Calls Overridable Method". I know this is happening, but my question is how to fix it so that the code still works and the message goes away.

public interface Foo{
   void doFoo();
}
public class FooImpl implements Foo{
 @Override{
 public void doFoo(){
    //.. Do important code
 }
}
public class Bar{
  private FooImpl fi;
  public Bar(){
    fi = new FooImpl();
    fi.doFoo(); // The message complains about this line
  }
}

Thanks!

Answer 1

As @Voo says,

your question is about calling a virtual method on an already completely constructed object. The well known downfalls of calling virtual methods on the to constructed object are well known, but don't apply here

From Effective Java 2nd Edition, Item 17: Design and document for inheritance, or else prohibit it:

There are a few more restrictions that a class must obey to allow inheritance. 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.

Invocation of an overridable method during object construction may result in the use of uninitialized data, leading to runtime exceptions or to unanticipated outcomes.

Constructors must invoke only methods that are final or private

You could use static factory methods to fix the problem that you have to create your objects from the Bar class.

Effective Java, Item 1: Consider static factory methods instead of constructors

The normal way for a class to allow a client to obtain an instance of itself is to provide a public constructor. There is another technique that should be a part of every programmer’s toolkit. A class can provide a public static factory method, which is simply a static method that returns an instance of the class.

So, you go to have the interface :

public interface Foo {
     void doFoo();
}

and the implementation:

public class FooImpl implements Foo {
   @Override
   public void doFoo() {
   //.. Do important code
   }
}

To create your class with your factory method you could work by this way:

• Use the interface to define the variable of your class private Foo fi instead of private FooImpl fi, using interfaces over concrete types is the key for good encapsulation and for loose coupling your code.

• Make your default constructor private to prevents instantiation of your class outside.

  private Bar() { // Prevents instantiation }

• Remove all calls to override methods that are present in your constructor.

• Create your static factory method

Finally you get a class Bar with a factory method like :

public class Bar {
    private Foo fi;

    private Bar() {// Prevents instantiation
        fi = new FooImpl();
    }

    public static Bar createBar() {
        Bar newBar = new Bar();
        newBar.fi.doFoo(); 

        return newBar;
    }
}

My Boss says: “the Sonar warnings are about symptoms, not about the disease. It’s best when you can treat the disease.” !

Answer 2

You could declare doFoo as final if you don't need to override that method later:

public final void doFoo() { }