Is this a bad pattern? (Switch inside for/foreach loop)

Question

I find myself writing code such as:

foreach($array as $key => $value) {
    switch($key) {
        case 'something':
            doSomething($value);
            break;
        case 'somethingelse':
            doSomethingElse($value);
            break;
    }
}

Is there a better way to go about this? Seems dirty to me, but I might just be over thinking it.

The only other alternative that I can think of is an if statement for each key, which doesn't seem any better. I.e. :

if($array[0] == 'something') {
    doSomething($array[0]);
}
if($array[1] == 'somethingelse') {
    doSomethingElse($array[1]);
}

(or something like that)

I can post exact code if needed, but this is the general outline of what happens. Please critique away, but remember that I'm looking for help here. So if I'm doing something egregiously wrong, then point it out.

Answer 1

Mapping your functions to keys in a dictionary/associative array is a common approach for this situation (as @jldupont has mentioned) -- not just in PHP but in many dynamic languages with associative arrays. For example, Python and Lua don't even have a switch statement -- this is pretty much the only way to emulate a switch.

Consider this approach:

<?
$arr[] = "bye";
$arr[] = "hi";

function sayHi() { print("Hello.\n"); }
function sayBye() { print("Goodbye.\n"); }

$funcs["hi"] = sayHi;
$funcs["bye"] = sayBye;

foreach($arr as $k){
    $funcs[$k]();
}

?>

Output:

Goodbye.
Hello.

It's overkill when you only have two distinct values, but obviously it becomes a more worthwhile approach as the number of situations you have to cover increases.

Answer 2

I tend to use the switch in the foreach loop. IMHO is less dirty than a bunch of if .

You can put your switch in a other function, like :

foreach($array as $key => $value) {
   doTransaction($key , $value);
}

...

function doTransaction($key, $value){
     switch($key) {
        case 'something':
            doSomething($value);
            break;
        case 'somethingelse':
            doSomethingElse($value);
           break;
    }
}

Answer 3

It's not a "bad" solution, but as always, there are alternatives. For instance, you could get rid of the switch statement and use an interpreted handler for the strings. This is similar to a list of function pointers, but you don't have to keep a list up to date to add new behavior; simply adding the new function to the handler will take care of it.

$array = array(
  "something" => "itsasecret",
  "somethingelse" => "i can't tell you",
);

class Handler {
  static function something($value) {
    printf("something: %s\n", $value);
  }

  static function somethingelse($value) {
    printf("somethingelse: %s\n", $value);
  }
}

$handler = new Handler();
foreach($array as $key => $value) {
  $handler->$key($value);
}

You'll probably need some code to sanitize the input strings and ensure that the method exists in your handler, but this might give you some ideas.