php Laravel - given a certain argument, call different function

Question

I am building a simple calendar with recurring events.

CalendarEvent.php

id
start_date

CalendarRecurringPattern.php

parent_id
pattern_type // daily, weekly, monthly, yearly ...

Using pattern_type i build each following occurrence starting from start_date, using a recursive function and Carbon::class. What bugs me is the function getNextOccurrence(), I'd like to refactor the if/else block because it's not very clean. Here it is:

use \Carbon\Carbon;
....
public function getNextOccurrence(Carbon $startDate) {
 $pattern_type = $this->getPatternType() // daily, weekly...
 if ($pattern_type == 'daily') {
 $startDate->addDay();
 } else if ($pattern_type == 'weekly') {
 $startDate->addWeek();
 } else if ($pattern_type == 'monthly') {
 $startDate->addMonth();
 } else if ($pattern_type == 'yearly') {
 $startDate->addYear();
 }
}

Thanks!

• Your Common Sense: the commonplace suggestion would be to use switch, but honestly I don't see what's wrong with a lot of nested if statements. Once I was eager to refactor this kind of code, but now I learned to value its verbosity and clean meaning, so I wouldn't change it into something more "elegant" but less readable
• Enrico: Thanks. I'm still growing as a developer and I'm doing my best to avoid bad habits. Recently I have been reading a lot about unit testing, a field in which I have no experience, and that function seemed not very clean from a testing perspective.

Answer 1

I don't personally see a problem with the way its currently structured as there aren't that many options, but you could do it a couple of other ways;

Option 1

Array function name mapping (hacky name I just came up with), you could do the following;

$functionMap = [
 "daily" => "addDay",
 "weekly" => "addWeek",
 "monthly" => "addMonth",
 "yearly" => "addYear"
];
if(!isset($functionMap[$pattern_type])){
 throw new \Exception("Can't find patern type $pattern_type", 1);
}
$startDate->$functionMap[$pattern_type]();

This might be seen as a sin by the PHP community but it works none the less

Option 2

You could use a switch which probably isn't a sin by the community.

switch ($pattern_type) {
 case "daily":
 // Do the daily task
 break;
 case "weekly":
 // Do the weekly task
 break;
 case "monthly":
 // Do the monthly task
 break;
 case "yearly":
 // Do the yearly task
 break;
 default: 
 throw new \Exception("Can't find pattern type $pattern_type", 1);
}

Conclusion

I would add some exception / error that can be picked up if a non matched pattern_type is passed.

I would also convert the "magic strings" E.G "daily" to constants in case you need to re-use, change or update them in the future!

In terms of line count;

• your way 10 lines of code
• option 1 11 lines of code
• option 2 15 lines of code

The switch & if statements may have some advantages when doing code analysis aswel where the expected required input values can be picked up.

• Enrico: Thanks for this. I'm wondering, why should the option n.1 be seen as a sin by the community? I find myself using the same concept many times with javascript.
• Dan: For example unit testing, unit testing would show this code as 100% tested even if you only tested one $pattern_type because there is only one path the code can take