Social Icons

Pages

Featured Posts

Wednesday 17 August 2016

Is Mastering the Art of 'Embracing the Change in Software Design' Too Difficult?

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:

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;
  }
}

Looks better than the previous approach.

Now, we will be able to handle new employee types and calculate hike/bonus for the newly added employee types. Further refactoring can be done in performAppraisal method to clearly specify how is hike computed, by moving  (getSalary() * getHikePercentage()) in a separate method.

While performing refactoring we tried to capture a few things:

  • Each employee type should be able to compute the salary using its own algorithm.
  • Code which is shared between Permanent and Contrator is moved up to the parent class.
  • Visibility of the methods and constructors was kept to minimum.
  • Magic numbers were given a name in form of a method. (Can be further refactored to use Constants).

[Assumption: Unit tests were present and were executed after each refactoring step]

I believe there are a few reasons we tend to write code which can not even speak for itself:

Lack of Awareness

I do not have an idea of what Clean Code is all about and I do not understand SOLID / DRY / KISS etc.

Mindset

I believe if/else is simple enough to understand and can be managed easily

Tendency to Ignore Code Smell(s)

I do not understand code smells and how does it impact me or my code. How much difference does it make to keep a method small or a single method which has 200 lines

Time Pressure

With the amount of time made available for development, I can not write the code which is understood by everyone. 

Design is Not Evolutionary

Once the design is done, it's done and can not be changed! Any change in the design is risky and can break everything. It is working properly in prod and should not be touched. 

I think we need to appreciate the fact that any piece of code is read more often than it is modified. We need to learn to criticize our own code, challenge our own development, understand various design principles, refactoring techniques and apply the same in development, learn to take small steps in development and accept the fact "Change is INEVITABLE".