Should I use a superclass' setters to initialise its instance variables from a subclass?

Question

I have a superclass 'Vehicle' and its subclass 'Car' as shown below. I want to initialise the variable 'wheels' from the constructor of the subclass Car. I can not set the variable via the super() call as it relies on a call to a method.

Is it safe to use the superclass' setter method here? If not is there a problem with the OO fundamentals of the relationship between the classes?

ps. try to excuse the clumsy vehicle/car/chassis example and assume that the method call 'findNumberOfWheels' outside of the constructor is necessary.

Vehicle Class

public abstract class Vehicle {

    private int wheels;
    private Chassis chassis;

    public Vehicle(Chassis chassis){
       this.chassis = chassis;
    }

    public int getWheels() {
       return wheels;
    }

    public void setWheels(int wheels) {
       this.wheels = wheels;
    }

    public Chassis getChassis() {
       return chassis;
    }

    public void setChassis(Chassis chassis) {
       this.chassis = chassis;
    }

}

Car Class

public class Car extends Vehicle{

    public Car(Chassis chassis) {
        super(chassis);
        setWheels(findNumberofWheels(chassis));
    }

    private int findNumberofWheels(Chassis chassis){
        return chassis.getWheels();
    }

}

That is ok, you don't need to rewrite the getter/setter on a sub-class while it will behave the same way. — Lucio, Mar 02 '15 at 02:08
What if somebody calls `Car.setWheels(70)`? You should avoid adding mutators to the interface of a base type if all possible. Injecting the number of wheels via the constructor and making the property `final` will be much safer. — 5gon12eder, Mar 02 '15 at 02:08
Check out what Jon Skeet has to say on the subject: http://stackoverflow.com/a/4893604/1980909 — Adrian Leonhard, Mar 02 '15 at 02:09
`findNumberOfWheels()` looks like a utility method to me, since it doesn't use any attributes of neither `Vehicle` nor `Car`. If this is the case in reality, then it should be declared static, either in the `Car` class or somewhere else. And if it's static, you can inline its invocation in the call to `super()` and pass it as an argument to the superclass' constructor, so I see no need to call the setter. — fps, Mar 02 '15 at 02:42
something seems to be wrong with this code - Chassis defines number of wheels `chassis.getWheels();`, so it is a delegation case. no point to maintain duplicate attribute in Car and Vehicle — harshtuna, Mar 02 '15 at 02:45
Thanks everyone. @Magnamag Your point about it being a utility method is very valid, and I agree it should be declared static. — dennyp, Mar 02 '15 at 03:56
@harshtuna You're right there is something fundamentally wrong with this code, still trying to get my head around it — dennyp, Mar 02 '15 at 03:56
@AdrianLeonhard Thanks for the reference, although my question is specifically about using setters of a superclass — dennyp, Mar 02 '15 at 03:59
@5gon12eder +1 for being careful about the use of mutators in base types — dennyp, Mar 02 '15 at 04:10

mk. · Answer 1 · 2015-03-02T02:15:23.877

1

If you're ok with constructing the superclass via the subclass, you should definitely be ok with using its setters.

I'm guessing that your alternative is to move those calls outside of your constructor, and call setWheels every time you instantiate a new Car? You definitely don't want to do that, because you would be repeating yourself, and you (or someone else) might forget to make the call. You should put as much of the initialization code as you can into the constructor. That way if you need to change it, you only need to change it in one place.

You can even set the superclass field to protected:

protected int wheels;

and change it directly. But that's up to your own judgement.

edited Mar 02 '15 at 02:15

answered Mar 02 '15 at 02:09

mk.

11,360
6
40
54

You're right, I don't want to move it outside the constructor as I would indeed forget to make the call. As @Magnamag pointed out 'findNumberofWheels()' is really a utility method to avoid putting the code directly in the constructor and so should be declared static. Thanks for your help. – dennyp Mar 02 '15 at 04:06

score 0 · Answer 2 · answered Mar 02 '15 at 15:57

findNumberOfWheels() looks like a utility method to me, since it doesn't use any attributes of neither the Vehicle nor the Car class.

If this is the case in reality, then it should be declared static, either in the Car class or somewhere else. And if it's static, you can inline its invocation in the call to super() and pass it as an argument to the superclass' constructor, so I see no need to call the setter.

In general, it's safer to initialize attributes via constructors and delegate to utility methods in case you need to make some calculation before assigning the variable.

If this is not possible, use either the factory or the builder pattern.

Should I use a superclass' setters to initialise its instance variables from a subclass?

2 Answers2