Skip to content
Advertisement

How to use the Open/Closed Principle to replace switch block that modifies shared state

I am working on a project that requires me to generate a report based on purchased rentals. I have to keep a count for each type of rental and sum the corresponding totals. Currently I am using a switch block to determine which action to take based on the current rental. However, it is my understanding that this violates the Open/Closed Principle, as I will have to modify the switch block every time a new rental is added. I would like to make this meet the OCP, but I am not sure how to go about. Below is my code:

public function generateReport($rentals)
{
    $type_1_count = 0;
    $type_2_count = 0;
    $type_3_count = 0;

    $type_1_sum = 0;
    $type_2_sum = 0;
    $type_3_sum = 0;

    foreach ($rentals as $rental) {
        switch ($rental->type) {
            case 'TYPE_1':
                $type_1_count++;
                $type_1_sum += $rental->price;
                break;
            case 'TYPE_2':
                $type_2_count++;
                $type_2_sum += $rental->price;
                break;
            case 'TYPE_3':
                // some of the rentals include other rentals which must be accounted for
                $type_1_count++;
                $type_1_sum += $rental->price / 2;

                $type_3_count++;
                $type_3_sum += $rental->price / 2;
                break;
            default:
                echo 'Rental Not Identified';
        }
    }
    return compact('type_1_count', 'type_2_count', 'type_3_count', 'type_1_sum', 'type_2_sum', 'type_3_sum');
}

I am modifying the shared state variables depending on the selected case. I reviewed many OCP examples but all of them show how to execute an action or return a value, but I need to modify the shared state instead. What would be the best way to refactor this code to be more inline with the OCP?

Advertisement

Answer

You can use an associative array. You just need to make sure the variable is set first. Something like this:

   $all_rentals = [];
   foreach ($rentals as $rental) {
       // make sure the variable has a default value
       if(!isset($all_rentals[$rental->type."_count"]) {
           $all_rentals[$rental->type."_count"] = 0;
           $all_rentals[$rental->type."_sum"] = 0;
       }
       $all_rentals[$rental->type."_count"]++;
       $all_rentals[$rental->type."_sum"] += $rental->price;
   }

...

This way you can add new values (rental types) without having to modify any existing code

User contributions licensed under: CC BY-SA
4 People found this is helpful
Advertisement