Exceptions thrown by a constructor
The architect reviewing my bowling example did not quiet like that my BowlingGame constructor threw an (checked) exception. My argument went, that it was there for mere convenience, to ease testing. With its help bowling games could be rolled wholesale, like so:@Test
public void neitherStrikeNorSpare() {
BowlingGame game = new BowlingGame(new int[] { 1, 2, 3, 4, 5, 1, 2, 3,
4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5 });
assertEquals(60, game.getScore());
}
He countered, this could also be achieved by a normal method. And right he is! Additionally, with such a method, rolls could be added to a game in several bulks. So lets re-factor the constructor from:
public BowlingGame(int[] rolls) throws BowlingException {
for (int roll : rolls) {
addRoll(roll);
}
}
into a «normal method» which the constructor calls:
public BowlingGame(int[] rolls) {
addRolls(rolls);
}
public void addRolls(int[] rolls) {
for (int roll : rolls) {
addRoll(roll);
}
}
Well, this some sleight of hand in that change. Where did the «throws BowlingException» declaration go? It already went during the last post in favor of an unchecked exception! The declaration is no longer required!
Notice something with the constructor? I did not, when I initially wrote this piece of code: the constructor called—and still calls—a non-final method! This is generally considered bad practice. If BowlingGame was sub-classed and addRoll/addRolls was overridden, then the constructor would call the sub-classes implementation although the sub-class constructor would have not been executed yet. That means the instance might not be in a usable state [Bloch, p. 89]! That makes one argument against using complex initialization within constructors: the temptation to call virtual methods.
As to further arguments against throwing exceptions in constructors—both Uncle Bob and Mr. Bloch give non. Most of the community on Stack Overflow seem to have no strong feelings against throwing exceptions from within constructors. Author and Stack Overflow veteran Jon Skeet, however, names two instances where one must be wary. The first is that the object under construction might escape immediate destruction and garbage collection:
It's possible for the «half-constructed» object to stick around though, if it's made itself visible earlier in the constructor (e.g. by assigning a static field, or adding itself to a collection).The second pitfall is less exotic. It is related to resources that require clean-up:
—Jon Skeet, «Can constructors throw exceptions in Java?», Stack Overflow
Because the caller (usually) will have no way of using the new object, the constructor ought to be careful to avoid acquiring unmanaged resources (file handles etc) and then throwing an exception without releasing them. For example, if the constructor tries to open a FileInputStream and a FileOutputStream, and the first succeeds but the second fails, you should try to close the first stream. This becomes harder if it's a subclass constructor which throws the exception…Both issues do not apply to the bowling constructor. The problem of calling a virtual method however must be fixed! There are various options: ditch the constructor altogether and call addRolls(int[]) instead on a freshly created instance. Alternatively, mark BowlingGame final thus preventing any sub-classes that could override addRolls(int[]). A little less drastic would be to just mark addRolls(int[]) and all the methods it calls recursively final`, which achieves the same effect. Or we can go for a static factory method:
—Jon Skeet, «Can constructors throw exceptions in Java?», Stack Overflow
public static BowlingGame fromRolls(int[] rolls) {
BowlingGame game = new BowlingGame();
game.addRolls(rolls);
return game;
}
Apart from being permitted to call virtual methods, it also improves readability at the call site. In the following code the purpose of the int array is not immediately clear:
@Test
public void neitherStrikeNorSpare() {
BowlingGame game = new BowlingGame(new int[] { 1, 2, 3, 4, 5, 1, 2, 3,
4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5 });
assertEquals(60, game.getScore());
}
Replaced by a static factory method, it is clear:
@Test
public void neitherStrikeNorSpare() {
BowlingGame game = BowlingGame.fromRolls(new int[] { 1, 2, 3, 4, 5, 1,
2, 3, 4, 5, 1, 2, 3, 4, 5, 1, 2, 3, 4, 5 });
assertEquals(60, game.getScore());
This is clearly the best way to go, in this case.
One questions remains: what about unit-tests for the newly created addRolls(int[]) method? I guess we are somewhat absolved from this obligation since the static factory method (and before that, the constructor) are already quiet thoroughly tested. If we opted to also test addRolls(int[]) the tests would be almost duplicated, with only a method call renamed. I do not know yet, if there is a pattern to efficiently write such test-cases.
