Java referencing final variable before is has been initialized

Question

I have this superclass Creature and its subclass Monster. Now I have this problem of a final variable being referenced without it being initialized.

public class Creature {

    private int protection;

    public Creature(int protection) {
        setProtection(protection);
    }

    public void setProtection(int p) {
        if(!canHaveAsProtection(p))
            throw new Exception();
        this.protection = p;
    }

    public boolean canHaveAsProtection(int p) {
        return p>0;
    }
}

and the subclass:

public class Monster extends Creature {

    private final int maxProtection;

    public Monster(int protection) {
        super(protection);
        this.maxProtection = protection;
    }

    @Override
    public boolean canHaveAsProtection(int p) {
        return p>0 && p<maxProtection
    } 
}

As you can see, when I initialize a new Monster, it will call the constructor of Creature with super(protection). In the constructor of Creature, the method canHaveAsProtection(p) is called, which by dynamic binding takes the overwritten one in Monster. However, this overwritten version uses the final variable maxProtection which hasn't been initialized yet... How can I solve this?

This code doesn't make any sense. Aren't you requiring that `maxProtection < maxProtection` here? — Keppil, May 22 '13 at 20:40
No, you're requiring the given protection in the constructor of Monster is smaller than maxProtection which still has to be set. — Confituur, May 22 '13 at 20:43
Yes, but `maxProtection` will be set to `protection` afterwards, so the logic is as I describe, but with the two steps reordered. — Keppil, May 22 '13 at 20:44

score 1 · Answer 1 · answered May 23 '13 at 03:52

Some points:

only Monster cares about a max value, so only it should know about this concept
all Creatures must have a protection > 0
don't defer range checking to a separate method
the upper and lower bound checking doesn't need to be in the same place
use the Decorator Pattern to solve the problem

Putting this all together, your code should look like this:

public class Creature {

    private int protection;

    protected Creature() {
    }

    public Creature(int protection) {
        setProtection(protection);
    }

    public void setProtection(int p) {
        if (p < 0)
            throw new IllegalArgumentException();
        this.protection = p;
    }
}

public class Monster extends Creature {

    private final int maxProtection;

    private Monster(int protection) {
        this.maxProtection = protection;
        setProtection(protection);
    }

    @Override
    public void setProtection(int p) {
        if (protection > maxProtection)
            throw new IllegalArgumentException();
        super.setProtection(p);;
    }

    public static Monster create(int protection) {
        Monster monster = new Monster(protection);
        monster.validate();
        return monster;
    }
}

You haven't shown what the validate() method dies, but if it's only needed for protection checking, I would delete it and the static factory method and make the constructor of Monster public.

score 0 · Answer 2 · answered May 22 '13 at 20:40

0

Monster does not extend Creature in the code you posted.

If it does, I see no reason for Monster to have a final variable. Creature ought to have the final variable, and Monster should simply access it. If there needs to be a max value validation for protection, it seems to me that all Creature instances ought to have it.

Push it up from the Monster to Creature and you're fine. You should have two arguments in the Creature constructor: protection and maxProtection. Throw an IllegalArgumentException if maxProtection < 0 or protection falls outside the range 0..maxProtection.

answered May 22 '13 at 20:40

duffymo

305,152
44
369
561

It's just a little part of the whole code which is more complex. The bottom line is: there's no reason for the variable maxProtection to be in the subclass. But if it's the only solution... – Confituur May 22 '13 at 20:41
I think it should be pushed up to the super class and made part of its contract. – duffymo May 22 '13 at 20:43
All the other subclasses of Creature also have the variable maxProtection then, which is of no use for them. Is that the proper way? – Confituur May 22 '13 at 20:45
1

I disagree - I think it is of use to them. It seems to me to be part of the contract for setting protection level, which should be true for all Creatures. I disagree with your thinking and design. – duffymo May 22 '13 at 21:25

score 0 · Answer 3 · answered May 22 '13 at 20:47

It's to do with your initialisation chain.

The child class must be fully I initialised before the parent can be initialised.

Currently, it looks like this...

Monster->Creature->Creature#setProtection->Monster#canHaveAsProtection ...

where maxProtextion hasn't been initialised, because the Creature constrcutor hasn't returned.

A solution would be to defer the initialisation some how, perhaps in a init method that the constructors can call

score 0 · Answer 4 · answered May 22 '13 at 20:50

It is really a bad practice to call public methods in constructor, especially when you expect to subclass it.

Your class should be solid and initialize it's state independently. I think you should set protection variable explicitly in constructor:

public Creature(int protection) {
    this.protection = protection;
}

If you really want to validate your parameters in constructor then extract common functionality to private methods:

public Creature(int protection) {
    Assert.isTrue(isProtectionValid(p));
    this.protection = protection;
}

private static boolean isProtectionValid(int p) {
    return p > 0;
}

public boolean canHaveAsProtection(int p) {
    return isProtectionValid(p);
}

score 0 · Answer 5 · answered May 22 '13 at 21:13

For the reasons mentioned in the other answers, do:

public Creature(int protection) {
    this.protection = protection;
}

public Monster(int protection) {
    super(protection + 1);
    this.maxProtection = protection;
}

That correction + 1 seems to be your intended logic because of the < maxProtection.

score 0 · Answer 6 · answered May 22 '13 at 21:14

0

How about you override the set protection en set the max first en than call on super set protection.

    @override
   public void setProtection(int p) {
        this.maxProtection = p;
    super.setProtection(p);
    }

answered May 22 '13 at 21:14

docDevil

72
4

Have you tried this one and did it work for you? I am just wondering! – docDevil May 23 '13 at 19:22

score 0 · Answer 7 · answered May 22 '13 at 21:42

You shouldn't call public methods in a constructor. Instead you could change constructor modifier to protected/private and use factory methods in which validation is called:

public class Creature {
    private int protection;

    protected Creature(int protection) {
        this.protection = protection;
    }

    public void setProtection(int protection) {
        if (!canHaveAsProtection(protection))
            throw new IllegalArgumentException();
        this.protection = protection;
    }

    public boolean canHaveAsProtection(int protection) {
        return protection > 0;
    }

    protected void validate() {
        if (!canHaveAsProtection(this.protection))
            throw new IllegalArgumentException();
    }

    public static Creature create(int protection) {
        Creature creature = new Creature(protection);
        creature.validate();
        return creature;
    }
}

And Monster:

public class Monster extends Creature {

    private final int maxProtection;

    private Monster(int protection) {
        super(protection);
        this.maxProtection = protection;
    }

    @Override
    public boolean canHaveAsProtection(int p) {
        // I changed '<' to '<=' because '<' wouldn't work anyway
        return p > 0 && p <= maxProtection;
    }

    public static Monster create(int protection) {
        Monster monster = new Monster(protection);
        monster.validate();
        return monster;
    }
}

Java referencing final variable before is has been initialized

7 Answers7