Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

avoiding type switching

If you're in a team and a programmer gives you an interface with create, read, update and delete methods, how do you avoid type switching?

Quoting Clean Code A Handbook of Agile Software Craftsmanship:

public Money calculatePay(Employee e)
    throws InvalidEmployeeType {
        switch (e.type) {
            case COMMISSIONED:
                return calculateCommissionedPay(e);
            case HOURLY:
                return calculateHourlyPay(e);
            case SALARIED:
                return calculateSalariedPay(e);
            default:
                throw new InvalidEmployeeType(e.type);
    }
}

There are several problems with this function. First, it’s large, and when new employee types are added, it will grow. Second, it very clearly does more than one thing. Third, it violates the Single Responsibility Principle7 (SRP) because there is more than one reason for it to change. Fourth, it violates the Open Closed Principle8 (OCP) because it must change whenever new types are added. But possibly the worst problem with this function is that there are an unlimited number of other functions that will have the same structure. For example we could have

isPayday(Employee e, Date date),

or

deliverPay(Employee e, Money pay),

or a host of others. All of which would have the same deleterious structure.

The book tells me to use the Factory Pattern, but in way that it makes me feel that I shouldn't really use it.

Quoting the book again:

The solution to this problem (see Listing 3-5) is to bury the switch statement in the basement of an ABSTRACT FACTORY,9 and never let anyone see it.

Is the switch statement ugly?

like image 210
Delirium tremens Avatar asked Dec 30 '22 05:12

Delirium tremens


1 Answers

In reality, the employee object should have its own calculate pay function that will give you the pay. This calculate pay function would change based on what type of employee it was.

That way it is up to the object to define the implementation, not the user of the object.

abstract class Employee
{
     public abstract function calculatePay();
}

class HourlyEmployee extends Employee
{
     public function calculatePay()
     {
          return $this->hour * $this->pay_rate;
     }
}

class SalariedEmployee extends Employee
{
     public function calculatePay()
     {
          return $this->monthly_pay_rate;
     }
}

When you build the Factory, THEN you do the switch statement there, and only once, to build the employee.

Lets say Employee was in an array, and the type of employee was held in $array['Type']

public function buildEmployee($array)
{
    switch($array['Type']){
       case 'Hourly':
            return new HourlyEmployee($array);
            break;
       case 'Salaried':
            return new SalariedEmployee($array);
            break;
}

Finally, to calculate the pay

$employee->calculatePay();

Now, there is no need for more than one switch statement to calculate the pay of the employee based on what type of employee they are. It is just a part of the employee object.

Disclaimer, I'm a minor, so I'm not completely positive on how some of these pays are calculated. But the base of the argument is still valid. The pay should be calculated in the object.

Disclaimer 2, This is PHP Code. But once again, the argument should be valid for any language.

like image 108
Tyler Carter Avatar answered Jan 13 '23 10:01

Tyler Carter