Kotlin calling non final function in constructor works

Question

In Kotlin, it warns you when calling an abstract function in a constructor, citing the following problematic code:

abstract class Base {     var code = calculate()     abstract fun calculate(): Int }  class Derived(private val x: Int) : Base() {     override fun calculate(): Int = x }  fun main(args: Array<String>) {     val i = Derived(42).code // Expected: 42, actual: 0     println(i) } 

And the output makes sense because when calculate is called, x hasn't been initialized yet.

This is something I had never considered when writing java, as I have used this pattern without any issues:

class Base {      private int area;      Base(Room room) {         area = extractArea(room);     }      abstract int extractArea(Room room); }  class Derived_A extends Base {      Derived_A(Room room) {         super(room);     }      @Override     public int extractArea(Room room) {         // Extract area A from room     } }  class Derived_B extends Base {      Derived_B(Room room) {         super(room);     }      @Override     public int extractArea(Room room) {         // Extract area B from room     } } 

And this has worked fine because the overriden extractArea functions don't rely on any uninitialized data, but they are unique to each respective derived class (hence the need to be abstract). This also works in kotlin, but it still gives the warning.

So is this poor practice in java/kotlin? If so, how can I improve it? And is it possible to implement in kotlin without being warned about using non-final functions in constructors?

A potential solution is to move the line area = extractArea() to each derived constructor, but this doesn't seem ideal since it's just repeated code that should be part of the super class.

Answer 1

The initialization order of a derived class is described in the language reference: Derived class initialization order, and the section also explains why it is a bad (and potentially dangerous) practice to use an open member in initialization logic of your class.

Basically, at the point when a super class constructor (including its property initializers and the init blocks) is executed, the derived class constructor has not yet run. But the overridden members retain their logic even when called from the super class constructor. This may lead to an overridden member that relies on some state, that is specific to the derived class, being called from the super constructor, which can lead to a bug or a runtime failure. This is also one of the cases when you can get a NullPointerException in Kotlin.

Consider this code sample:

open class Base {     open val size: Int = 0     init { println("size = $size") } }  class Derived : Base() {     val items = mutableListOf(1, 2, 3)     override val size: Int get() = items.size } 

^{(runnable sample)}

Here, the overridden size relies on items being properly initialized, but at the point when size is used in the super constructor, the backing field of items still holds null. Constructing an instance of Derived therefore throws an NPE.

Using the practice in question safely requires considerable effort even when you don't share the code with anyone else, and when you do, other programmers will usually expect open members to be safe to override involving the state of the derived class.

---

As @Bob Dagleish correctly noted, you can use lazy initialization for the code property:

val code by lazy { calculate() } 

But then you need to be careful and not use code anywhere else in the base class construction logic.

Another option is to require code to be passed to the base class constructor:

abstract class Base(var code: Int) {     abstract fun calculate(): Int }  class Derived(private val x: Int) : Base(calculateFromX(x)) {     override fun calculate(): Int =          calculateFromX(x)      companion object {         fun calculateFromX(x: Int) = x     } } 

This, however, complicates the code of the derived classes in cases when the same logic is used both in overridden members and for calculating the values passed to the super constructor.

Answer 2

It is definitely poor practice because you are invoking calculate() on a partially constructed object. This suggests that your class has multiple phases of initialization.

If the result of calculation() is used to initialize a member, or perform layout or something, you might consider using lazy initialization. This will defer the calculation of the result until the result is really needed.