Why is calling setters from constructors discouraged in Java?

In Java, coders are discouraged from calling setter functions in class constructors. Even though doing so can sometimes reduce the amount of repeated code.

Consider the following class:

Unit.java

public class Unit {
	protected int health;

	public Unit(int hp) {
		// Prevents health for being set to 0 or less.
		if(hp <= 0)
			throw new IllegalArgumentException("Health must be more than 0.");
		health = hp;
	}

	public void setHealth(int hp) {
		// Prevents health for being set to 0 or less.
		// Repeat of the code in the constructor.
		if(hp <= 0)
			throw new IllegalArgumentException("Health must be more than 0.");

		health = hp;
	}

}

Instead of doing the check twice across 2 functions to ensure the incoming hp value is correct, it might occur to some coders that we can call the setter within the constructor instead, to reduce the amount of repeated code:

Unit.java

public class Unit {
	protected int health;

	public Unit(int hp) {
		// Prevents health for being set to 0 or less.
		if(hp <= 0)
			throw new IllegalArgumentException("Health must be more than 0.");
		health = hp;
		setHealth(hp);
	}

	public void setHealth(int hp) {
		// Prevents health for being set to 0 or less.
		// Repeat of the code in the constructor.
		if(hp <= 0)
			throw new IllegalArgumentException("Health must be more than 0.");

		health = hp;
	}
}

This, however, is discouraged, because (according to textbooks) setters like setHealth() can be overriden by child classes, creating unexpected or buggy behaviour in these child classes.

What does this mean?

Let’s use the Unit class above to illustrate this. We are going to have a HandicappedUnit class inherit from Unit, to represent a class of units whose health values can be multiplied by a handicap factor — a feature you see a lot in older fighting games.

HandicappedUnit.java

public class HandicappedUnit extends Unit {

	protected double handicap;

	public Hero(int hp, double handicapFactor) {
		super(hp); // Compulsory call to the superclass constructor.
		handicap = handicapFactor;
	}

	@Override
	public void setHealth(int hp) {
		// Changes the way the health is set.
		health = health * handicapFactor;
	}

	public int getHealth() { return health; }
}

Due to the Unit class calling the setHealth() setter in its constructor, running the following code will always result in the HandicappedUnit having a health of 0.

If you want to try this out, you can copy the collated code below and run it on an online Java compiler.

UnitGame.java

public class UnitGame {
	public static void main(String[] args) {
		// Set the unit's health to 100, with a handicap of 0.5.
		HandicappedUnit hu = new HandicappedUnit(100, 0.5);

		// Expected value: 50; Actual value: 0
		System.out.println(hu.getHealth());
	}
}

This happens simply because when health is set in HandicappedUnit, the handicap value is still unset (i.e. in its default value of 0), and it is unset because:

1. When the constructor for HandicappedUnit is called, it is compulsory for the class to first call its super-class constructor (i.e. super(), which calls the Unit constructor). This means that Unit‘s constructor runs first.
2. Unit‘s constructor calls setHealth() to set the health attribute, but because HandicappedUnit overrides the method with its own, HandicappedUnit‘s setHealth() is called instead.
3. Because handicap is not yet set (since we are still in Unit‘s constructor), the health of the HandicappedUnit is set to 100 (health) × 0 (handicap) = 0.
4. Unit‘s constructor finishes running, and HandicappedUnit‘s constructor continues running, setting handicap to 0.5.

TL;DR: The newly-created HandicappedUnit has its health set to 0 because its health is set before the handicap value is set, and this is caused by the setHealth() method being overriden.

---

Article continues after the advertisement:

See discounted assets

---

What’s the recommended practice?

As far as I’m concerned, it isn’t a hard rule that setters should not be called in constructors. However, if you’re doing it without awareness of its potential ramifications, it can create bugs or inconsistencies that can be difficult to fix.

As a best practice, if you want to call setters in a class constructor, you’re recommended to mark the setters as final, so that child classes will not be able to override them:

Unit.java

public class Unit {
	protected int health;

	public Unit(int hp) {
		setHealth(hp);
	}

	public final void setHealth(int hp) {
		// Prevents health for being set to 0 or less.
		// Repeat of the code in the constructor.
		if(hp <= 0)
			throw new IllegalArgumentException("Health must be more than 0.");

		health = hp;
	}
}

Conclusion

Do you have a different opinion on this issue? Feel free to share it with us in the comments below.

For those of you who want to test out the HandicappedUnit code, you can copy the collated script below, and run it on this online Java compiler.

UnitGame.java

public class UnitGame {
	public static void main(String[] args) {
		// Set the unit's health to 100, with a handicap of 0.5.
		HandicappedUnit hu = new HandicappedUnit(100, 0.5);

		// Expected value: 50; Actual value: 0
		System.out.println(hu.getHealth());
	}
}

class Unit {
	protected int health;

	public Unit(int hp) {
		setHealth(hp);
	}

	public void setHealth(int hp) {
		// Prevents health for being set to 0 or less.
		if(hp <= 0)
			throw new IllegalArgumentException("Health must be more than 0.");

		health = hp;
	}
}

class HandicappedUnit extends Unit {

	protected double handicap;

	public Hero(int hp, double handicapFactor) {
		super(hp); // Compulsory call to the superclass constructor.
		handicap = handicapFactor;
	}

	@Override
	public void setHealth(int hp) {
		// Changes the way the health is set.
		health = health * handicapFactor;
	}

	public int getHealth() { return health; }
}