Change is inevitable and we're supposed to be able to change on the fly, but is that too difficult?
We as software developers often find ourselves in situations where making a small change in the code becomes a nightmare for us. Though we acknowledge the fact that "Change is inevitable", we haven't been able to master the art of changing the Code and or the design with ease.
Why? Have we neglected the design principles, or have we rejected the thought of evolving the design, or have we ignored the refactoring hints which we might have seen during development?
Well, it is difficult to answer but it is a mix of everything. Not believing that our design does evolve, ignoring refactoring hints, not challenging ourselves to find better solutions, etc.
Let's try and see if we can find any answer:
public class Employee{
private EmployeeTypeE employeeType;
public employee(EmployeeTypeE employeeType){ //other details ommitted
this.employeeType = employeeType;
}
public EmployeeTypeE getEmployeeType(){
return employeeType;
}
}
public class PayRollService(){
public double performAppraisal(final Employee employee){
double salary = employee.getSalary();
if ( employee.getEmployee() == EmployeeTypeE.CONTRACTOR ){
salary = salary + (salary) * 0.1;
}
return salary;
}
}
At this stage of development, some code smells are visible:
- Does an employee need to provide a getter for EmployeeTypeE?
- Should PayRollService be asking for the salary ? (TELL DON'T ASK?)
- salary = salary + (salary) * 0.1 --> salary is neither a collecting variable not a looping variable, but is re-assigned.
- peformAppraisal is using most of the data of Employee class. Should this method not move to Employee?
But, we as developers choose to ignore them. No Refactoring! Change is inevitable! And a change comes in. "We need to return the new salary and bonus for CONTRACTOR and EMPLOYEE."
public class PayRollService(){
public double[] performAppraisal(final Employee employee){
double salary = employee.getSalary();
double bonus = 0.0;
if ( employee.getEmployee() == EmployeeTypeE.CONTRACTOR ){
salary = salary + (salary) * 0.1;
}
else if ( employee.getEmployee() == EmployeeTypeE.EMPLOYEE ){
salary = salary + (salary) * 0.4 + calcuationBasedOnRatingReceived();
bonus = 1_00_000;
}
return new double[] {salary, bonus};
}
}
And, the effective change goes this way:
public class PayRollService(){
public double[] performAppraisal(final Employee employee){
double salary = employee.getSalary();
double bonus = 0.0;
if ( employee.getEmployee() == EmployeeTypeE.CONTRACTOR ){
salary = salary + (salary) * 0.1;
}
else if ( employee.getEmployee() == EmployeeTypeE.EMPLOYEE ){
salary = salary + (salary) * 0.4 + calcuationBasedOnRatingReceived();
bonus = 1_00_000;
}
return new double[] {salary, bonus};
}
}
We decided to:
- Use an array for returning multiple values.
- Use another simple if condition to handle the calculation for EMPLOYEE
- Use magic number 1_00_000, letting the readers of the code guess what and why of 1_00_000
- We had options and we probably chose the most illogical of all.
We could have attempted:
- State or Strategy to handle EmployeeType
- Making Employee an abstract class
- Creating an abstraction say, PerformanceAppraisal containing a hike and bonus
abstract class Employee {
public abstract PerformanceAppraisal performAppraisal();
protected double getHikePercentage(){
return 0.0;
}
protected double getDefaultBonus(){
return 0.0;
}
protected double getSalary() { return salary; }
//static method to instantiate appropriate employee or a separate factory class
}
public class PermanentEmployee extends Employee {
protected PermanentEmployee() {
}
public PerformanceAppraisal performAppraisal() {
double hike = getSalary()
+ (getSalary() * getHikePercentage())
+ calcuationBasedOnRatingReceived();
return new PerformanceAppraisal(hike, getDefaultBonus());
}
protected double getHikePercentage() {
return 0.04;
}
protected double getDefaultBonus(){
return 1_00_000;
}
}
public class ContractorEmployee extends Employee {
protected ContractorEmployee() {
}
public PerformanceAppraisal performAppraisal() {
double hike = getSalary() + (getSalary() * getHikePercentage());
return new PerformanceAppraisal(hike, getDefaultBonus());
}
protected double getHikePercentage(){
return 0.01;
}
}
