Best practice for subclass/superclass fields

Question

I am having trouble deciding between these three ways to handle field variables for a subclass and superclass.

Method 1:

public abstract class Vehicle {
    public abstract int getNumberOfWheels();
    public abstract int getCost();
}

public class Car extends Vehicle {
    private int numberOfWheels;
    private int cost;

    public Car() {
        this.numberOfWheels = 4;
        this.cost = 10000;
    }

    public int getNumberOfWheels() {
        return numberOfWheels;
    }

    public int getCost() {
        return cost;
    }
}

With this method i have to implement the same duplicate getter methods in every subclass of Vehicle. I imagine this would be a problem with more complicated getter method, that have to be duplicated and eventually maintained.

Method 2:

public abstract class Vehicle {
    private int numberOfWheels;
    private int cost;

    public int getNumberOfWheels() {
        return numberOfWheels;
    }

    public int getCost() {
        return cost;
    }

    public void setNumberOfWheels(int numberOfWheels) {
        this.numberOfWheels = numberOfWheels;
    }

    public void setCost(int cost) {
        this.cost = cost;
    }
}

public class Car extends Vehicle {
    private int numberOfWheels;
    private int cost;

    public Car() {
        super.setNumberOfWheels(4);
        super.setCost(10000);
    }
}

With this method i have to implement setter methods that i might not want to have. I might not want other classes to be able to change the fields, even in the same package.

Method 3:

public abstract class Vehicle {
    private int numberOfWheels;
    private int cost;

    public class Vehicle(int numberOfWheels, int cost) {
        this.numberOfWheels = numberOfWheels;
        this.cost = cost;
    }

    public int getNumberOfWheels() {
        return numberOfWheels;
    }

    public int getCost() {
        return cost;
    }
}

public class Car extends Vehicle {
    private int numberOfWheels;
    private int cost;

    public Car() {
        super(4, 10000);
    }
}

With this method and with a lot of fields, the amount of constructor parameters will grow huge, which just feels wrong.

It seems like this would be a common enough problem for there to exist some kind of "best practice". Is there a best way to do this?

the private fields in `Car` are superfluous if they are declared in `Vehicle`. You also may want to investigate "[protected](http://stackoverflow.com/a/215505/982149)" access modifier. — Fildor, Dec 05 '16 at 12:07

GhostCat · Answer 1 · 2016-12-05T12:24:26.190

Several thoughts here:

It is actually good that you don't get into the protected fields story; one should avoid sharing fields between base/extending classes if possible.
Similarly, it is also good practice to avoid using setters. That kinda rules out your second options.

Further on ...

You could rewrite your subclass in option 1 to:

@Override // always use that when OVERRIDING methods!
public int getNumberOfWheels() { return 4; }

In your option 1, those numbers are actually constants as your code is not showing any means to change those values. So there is no need to use fields in the derived classes! Unless of course, you might imagine different types of Cars, and a need to allow for 3 or 5 wheels as well. In that case, you would offer a default ctor; and one that takes that number-of-wheels (to then store it in some final property).

Then: you are correct, when a lot of information would be required, that option3 using constructors will "blow up" on you. But: that would be just a consequence of a design problem anyway. Because: one should be conservative about number of fields anyway. Meaning: if your class carries so many fields that init'ing them via constructors looks like a problem, than that is an indication that you got too many fields in the first place! In such a situation, you would look into your model to figure which properties really belong into your class.

Example: in your code, you are representing "cost" as property of your base class. But is that really true? Is its "prize" really an essential property of any vehicle? What I mean is: a car is just a car; it doesn't "care" about its value. That value is some extrinsic property, that other systems would impose on that car. Meaning: a vehicle doesn't necessarily need a price/cost property. You only start thinking about that when vehicles are entities in some bigger context that deals with the values of its entities. So some other EntityManager thingy might be a better place to keep track of vehicles and their corresponding (current) value.

"constants as your code is not showing any means to change those values" ... well a car mostly is defined as a vehicle with 4 wheels. Though there are cars with 3 ... So maybe one should consider a (constant) default value of "4" that could be changed through an optional CTOR-Param? But the point is that a specific (type of) car wouldn't change its wheel-count over its lifetime, right? — Fildor, Dec 05 '16 at 12:19
God point; I enhanced my answer accordingly. And you are right; I assume that the number of wheels would be fixed; a change in that number would probably indicate end-of-lifetime for the corresponding car ;-) — GhostCat, Dec 05 '16 at 12:25

aUserHimself · Answer 2 · 2016-12-05T13:25:01.160

Good practice is rather relative here; in your case, it depends on what you are trying to achieve.

Will any subclass of Vehicle have a number of wheels and a cost associated? If the answer is Yes, then it is good practice to add them to the superclass. If you may have TrackVehicle as subclass, then numberOfWheels is not applicable here and hence does not belong in the superclass.
Ask yourself the question: do you really need setters? Will you have to change the state of your instance after creation? If not, don't add them: you can create a constructor in the superclass that takes the total number of required parameters and use it in every subclass:
```
public Car(int numberOfWheels, int cost) {
    super(numberOfWheels, cost);
}
```

By trying to guess your intention, this would be my method of doing it:

public abstract class Vehicle {
    private int numberOfWheels;
    private int cost;

    public Vehicle(int numberOfWheels, int cost) {
        this.numberOfWheels = numberOfWheels;
        this.cost = cost;
    }
    public int getNumberOfWheels() {
        return numberOfWheels;
    }
    public int getCost(){
        return cost;
    }
}

and a specific subclass where every Car has 4 wheels and a cost for the outside world that is actually much bigger than the initial one (just to show the fact that you can override a method if required, no need to duplicate it)

public class Car extends Vehicle {
    public Car(int cost) {
        super(4, cost);
    }
    @Override
    public int getCost(){
        return cost * 2;
    }
}

About the 'constructor parameters will grow huge' problem: have a look at the 'Builder' design patters. (Effective Java - Builder pattern)

Timothy Truckle · Answer 3 · 2020-03-20T11:31:57.697

I am having trouble deciding between these three ways to handle field variables for a subclass and superclass.

In the first place you should prefer composition over inheritance which means that concrete classes do not inherit from each other, only interfaces.

Beside this your question somehow depends on the purpose of your classes.

Classes can either be "plain value classes" without any business logic (aka data transfer objects - DTOs) or "regular" objects.

DTOs

When you're desining DTOs you should create them as beans which means that you should create public getter methods for each property. By any chance you should make your DTOs immutable which means all member variables are declared with the final keyword. Then you have to set the values via constructor.

However: some frameworks require DTOs with default constructors and setters for the member variables.

regular objects

In all other classes you should not provide access to member variables of a class neither directly nor via getters/setters. This would violate the most important OO principle: information hiding aka encapsulation.

Initial values should be set via constructor and when you have the need to modify a member value you should provide methods with business related names.

eg.:

class Vehicle {
  private int speedInMph;
  private final int maximumSpeedInMph;
  public Vehicle(int initialSpeedInMph, int maximumSpeedInMph){
    this.speedInMph=initialSpeedInMph;
    this.maximumSpeedInMph=maximumSpeedInMph;
  }
  public void accelerateBy(int accelerationInMph){
    this.speedInMph+=accelerationInMph;
    if(maximumSpeedInMph<this.speedInMph)
       this.speedInMph=maximumSpeedInMph;
  }

  public void decelerateBy(int decelerationInMph){
    this.speedInMph-=decelerationInMph;
    if(0>this.speedInMph)
       this.speedInMph=0;
  }
}

Best practice for subclass/superclass fields

3 Answers3

DTOs

regular objects