I have an internal application in which one function is containing too many switch cases.
This is developed in php.This specific function is used to write changes to database and also to keep a history of individual field values. So what it does is have a case for each field as different things needs to be applied for different fields.
switch ($item){
case 'item1':
do_something();
case 'item2':
do_something_different():
}
Is there a design pattern to follow in such cases. A function for each item doesn't look so future proof either.
Update: pastebin link
That's just not a good function. It should be three functions, edit_name
, edit_manager
and edit_liscencedata
. you can move all the stuff that repeats between cases into the constructor of the Change
class that you should define.
You're not really revealing a lot about your code with that sample, but another option would be to use a state machine. You can take an array and assign the item names as keys and the values would be the function.
$machine = array('item1' => do_something, 'item_2' => do_something_different);
$machine['item1']();
Then it's even open for pluggability where you can include a file that says:
$machine['item3'] = do_something_else_else;
From the code snippet provided it's obvious you're facing quite a severe case of intermingled responsibilities and copy paste coding from lack of factoring. Your edit function handles stuff on at least three different layers and for three distinct objects. The refactoring operations I would be likely to apply are extracting Validator classes for each of the different sets of data you're handling, Model classes likewise for interacting with the database in each case and Commands to bind the three together: accept data, pass it through validators, then interact with their respective Models to persist the data. The Models would then generate Change entries based on successful operations.
If you're disinclined to take on such significant change right now, good first steps would be separating the different parts in your edit function into other, better partitioned functions and only worry about classes and polymorphism when you've tackled that first hurdle. To better understand why your code is broken and what you can do to mend it, I recommend reading something like Refactoring (by Martin Fowler) or perhaps The Pragmatic Programmer.
Just a thought, but instead of the switch statement, you might want to have a sort of hook system available for this ...
so, for a list of possible stuff like
item1, item2, ..., itemN
you might try something like
function item1action ( ) { /* ... */ }
function item2action ( ) { /* ... */ }
function itemNaction ( ) { /* ... */ }
and have some sort of default method like
function itemDefaultAction ( ) { /* ... */ }
so that for the previous switch statement, you could instead do
$function = "{$item}action";
if (! function_exists($function)) {
return itemDefaultAction();
}
call_user_func($function);
return call_user_func($function)