CODEWITHSUNDEEP
javavariablesinheritance
Meena Chaudhary
Meena Chaudhary

Reputation: 10665

Right approach for setting variables in super type

I have an abstract class Compartment with variables label tariff totalSeats and seatsAvailable as shown below. I am setting these variables by calling super() from the constructor of subclass FirstClassCompartment. I have few questions regarding my code.

First of all, is this a right approach to set variables using super class constructor or I should create a setter method in super class to set these variables.

Second, is it bad coding to access the variables of super class directly and not using getter/setter methods.

Last, Value of variable seatsAvailable in super class depends upon the variable seatsReserved in sub class like this seatsAvailable = totalSeats - seatsReserved. Might this cause code maintenance or flexibility issues in the future?

public abstract class Compartment {

    protected final char label;
    protected final double tariff;
    protected final int totalSeats;
    protected int seatsAvailable;

    public Compartment(char label, double tariff, int totalSeats) {
        this.label = label;
        this.tariff = tariff;
        this.totalSeats = totalSeats;
    }

    public char getLabel() {
        return label;
    }

    public int getTotalSeats() {
        return totalSeats;
    }

    public void setSeatsAvailableLessByOne() {
        seatsAvailable--;
    }

    public abstract int getSeatsAvailable();

    public abstract double getTariff();

}



public class FirstClassCompartment extends Compartment {

    protected final int seatsReserved;

    public FirstClassCompartment(char label, double tariff, int totalSeats, int seatsReserved) {
        super(label, tariff, totalSeats);
        this.seatsReserved = seatsReserved;
        seatsAvailable = totalSeats - seatsReserved;
    }

    @Override
    public double getTariff() {
        return tariff + tariff * .13;
    }

    @Override
    public int getSeatsAvailable() {
        return seatsAvailable;
    }
}

Upvotes: 0

Views: 106

Answers (3)

Ezequiel
Ezequiel

Reputation: 3592

I use to declare my fields with 'package' visibility. Its the default visibility, it can be accessed by inhered classes and same package classes. It helps a lot when you are unit testing, because test are written in the same package.

Indiscriminate use of getters/setters breaks encapsulation in my opinion. You are exposing all by fields default, when probably its not needed.

If your variables are setted only one time in constructor, then they should be 'final'.

If a setter has some logic, and in a future you may need to override it, then make it overridable, but its not a good practice to use an overridable method inside a constructor.

Upvotes: 0

chiastic-security
chiastic-security

Reputation: 20520

First question: this is absolutely the right way to do it. Invoke the superclass constructor, and pass the relevant arguments.

Second question: this is in part a stylistic thing. It's good practice not to make the fields directly available to all and sundry, so you wouldn't want them to be public, but having them available to your own subclasses is not disastrous. It would be quite reasonable to make them available via getters and setters, though, and certainly if you had fields that you wanted to be readable by the subclass but not writable, then a getter would be appropriate.

Third question: there's no problem here. The superclass is abstract, so you'd expect the subclass implementation to fill in some of the gaps. But since the seatsAvailable logic is so simple (a subtraction), you might think about getting rid of the seatsAvailable field entirely, and just having

@Override
public int getSeatsAvailable() {
    return totalSeats - seatsReserved;
}

That way, it will be calculated on the fly, and you won't have to worry about keeping this field in sync with the others.

Upvotes: 1

Mikkel Løkke
Mikkel Løkke

Reputation: 3749

Looks good to me, I would probably rename the method setSeatsAvailableLessByOne to something more meaningful, like decrementSeatsAvailable if you want to have it at all.

Edit You're right that having a seatsAvailable is redundant. You could just move seatsReservedand the derived value to the super class tho'. It's unlikely that there would be other ways to implement the getSeatsAvailablemethod, and even if there was, you could still override it, in these specific cases.

Upvotes: 0

Related Questions