Problem
I’m making my first steps in OOP and I’m currently doing some exercises. I’ve done a simple program with class structure like this (I’ll provide only essential parts):
Super class:
public abstract class Employee {
private String lastName;
private float salary;
public Employee(String lastName, float salary) {
this.lastName = lastName;
this.salary= salary;
}
}
And several child classes, all similar to this:
public class Worker extends Employee {
private float hourRate;
public Worker (String lastName, float salary, float hourRate) {
super(lastName, salary);
this.hourRate = hourRate;
}
}
My concern is, when I’ll decide to add some field to the Employee class that have to be set in constructor, let’s say firstName, than I have to add it there and in constructor’s arguments of all child classes and where I call super() inside them. It doesn’t seem to be the best solution… Is there a better way to do such a thing? Maybe there is some pattern I don’t know yet?
Solution
Short answer: no, there’s no better way.
There’s also nothing wrong with this way of working. You’re adding a field that every instance of this class needs to have for your program to work correctly. That’s a good reason to enforce initialising it in the constructor. Your compiler (or even better, your IDE) will tell you where else you need to update your code.
In case you’re developing a package that other people use where they need to have some time to adjust to the new version you need to be more careful when making API changes (like adding parameters to a public constructor).
In that case you make the current constructor @deprecated and provide a new one next to it. You can also provide a default value for the new field so that the original constructor still “works”. For example:
public class Person {
private String lastName;
private String firstName;
@deprecated
public Person(String lastName) {
this("", lastName);
}
public Person(String firstName, String lastName){
this.firstName = firstName;
this.lastName = lastName;
}
...
This way, their IDE will still tell them they’re using an outdated constructor, but at least they’ll still be able to run their current code without changing all the calls from the start.
Alternatively you could look into other options besides adding the fields to that class. For example, you could have a lookup class that, given a certain employee, looks up his salary in a table for example. This greatly depends on what your program is supposed to do. With the little information you’ve given us here it’s hard to propose concrete solutions like this.
There is no “better” way.
- You can remove them form the
Employeeconstructor and rely on setters - You can use a builder
- You can think of composition instead of extension
But if you want to be sure that all required parameters are given when creating a child. You there is no other way.
First of all, a bug fix: the properties of Employee that are to be shared with the sub classes should be defined as protected and if they are to be accessed by client classes, they need to be public (or have public getter methods)
now for the answer: you can (and should) have separate setter method for each property. this will allow to create concrete Employee instances and then assign any number of properties after constructor is finished.
public abstract class Employee {
protected String firstName;
protected String lastName;
protected float salary;
public Employee(String lastName, float salary) {
setLastName(lastName); // constructor should call setters as well
setSalary(salary); // this is in case setter is doing some logic
}
public void setFirstName(String firstName) {
this.firstName = firstName;
}
public void setLastName(String lastName) {
this.lastName = lastName;
}
public void setSalary(float salary) {
this.salary = salary;
}
}
public class Worker extends Employee {
...
}
public class EmployeeCreator {
public void createWorker() {
Employee worker = new Worker(); // assuming there is an empty constructor
worker.setFirstName("John");
worker.setLastName("Doe");
worker.setSalary(100.00F);
((Worker)worker).setHourRate(10.00F);
}
}
Now, this puts the responsibility of creating complete valid Employees on the client class. If you want to be able to verify that the new Employees has all mandatory properties at the time of instance creation, you can utilize the Builder pattern and have the build() method validate that all properties were assigned:
public abstract class Employee {
protected String firstName;
protected String lastName;
protected float salary;
public Employee() {}
public Employee(String lastName, float salary) {
setLastName(lastName); // constructor should call setters as well
setSalary(salary); // this is in case setter is doing some logic
}
public String getFirstName() {
return firstName;
}
public void setFirstName(String firstName) {
this.firstName = firstName;
}
public String getLastName() {
return lastName;
}
public void setLastName(String lastName) {
this.lastName = lastName;
}
public float getSalary() {
return salary;
}
public void setSalary(float salary) {
this.salary = salary;
}
public boolean isValid() {
return firstName != null && lastName != null && salary > 0;
}
public static final class Builder {
private Employee newEmployee = null;
public Builder newWorker() {
newEmployee = new Worker();
return this;
}
public Builder withFirstName(String firstName) {
newEmployee.setFirstName(firstName);
return this;
}
public Builder withLastName(String lastName) {
newEmployee.setLastName(lastName);
return this;
}
public Builder withSalary(float salary) {
newEmployee.setSalary(salary);
return this;
}
public Builder withHourRate(float hourRate) throws UnsupportedOperationException {
if (newEmployee instanceof Worker) {
((Worker)newEmployee).setHourRate(hourRate);
} else {
// cannot call this for any Employee other than Worker
throw new UnsupportedOperationException();
}
return this;
}
public Employee build() throws IllegalStateException {
if (newEmployee.isValid()) return newEmployee;
// new enployee not fully assigned
throw new IllegalStateException();
}
}
}
public static class Worker extends Employee {
...
public boolean isValid() {
return super.isValid() && hourRate > 0;
}
}
public static class EmployeeCreator {
public void createWorker() {} {
Employee worker = new Employee.Builder()
.newWorker()
.withFirstName("John")
.withLastName("Doe")
.withSalary(100.00F)
.withHourRate(10.00F).build(); // will throw IllegalStateException if instance not assigned correctly
}
}
There is another way. It does not remove the need to do this, but it can significantly reduce it.
Use composition.
Instead of Employee containing directly information about person, it can contain object person.
Then adding another field with information about person is no longer change of employee and therefore does not necesitate need to change its descendants.
And Person can enforece invariants about person and Employee will now just enforce that it actually must have Person.