How many cases in the switch() function is too many?

cruz610

Because right now I would use about 36...heh...thats a lot of cases...is that too many or is it fine?

ahundiak

From a technical point of view it's fine.

But sometimes having that many cases indicates a problem with the application design.

cruz610

Yes, I understand its a possible application weakness, however I simply can think of no other way to do it. Its a pretty complex survey script that has 36+ steps and there is no pretty way to deal with something like that 🙁 The cases simply include the required files for the survey depending on what step the user is on, I just dont want them to leave the single page.

ahundiak

Have you considered storing the include file names in an array and then simply looking them up with the step number?

Or could you actually name the files based on the step number and then calculate the name on the fly?

Just thoughts. A switch would work fine.

cruz610

Yea, I was thinking about using an array. Right now I have it include a file for EVERY case

	case 'u_ben_who':
		include("../survey_templates/u_ben_who.php");
		break;
	case 'u_ben_receiveatonce':
		include("../survey_templates/u_ben_receiveatonce.php");
		break;
	case 'u_ben_howpaid':
		include("../survey_templates/u_ben_howpaid.php");
		break;

etc...but I was thinking about doing something like:

	case 'u_ben_who':
	case 'u_ben_receiveatonce':
	case 'u_ben_howpaid':
		include("../survey_templates/$_GET['page'].php");
		break;

would that work too?

ahundiak

Reading a filename from GET data can be a bit dangerous since the user might change the name and see something they shouldn't. Restricting it to a directory is good. I'd also make sure they didn't add anything like ../../secret.php

cruz610

Yes, thats why I would make sure that it was in one of the cases. I originally just had it as if (isset($POST['page'])) include ('../survey_templates/$POST['[page'].php"); but that proved to be WAY too dangerous 😉

weekender

like ahundiak said, you could put it into an associative array

$files = array("u_ben_who" => "../survey_templates/u_ben_who.php",
"u_ben_receiveatonce" => "../survey_templates/u_ben_receiveatonce.php");

or, to shorten, use a path var

$path = "../survey_templates/";
$files = array("u_ben_who" => "u_ben_who.php", "u_ben_receiveatonce" => "u_ben_receiveatonce.php");
$file = $path . $files[option];

or even shorter

$file = $path . $option . ".php";

Weedpacket

Originally posted by cruz610
Yes, thats why I would make sure that it was in one of the cases. I originally just had it as if (isset($POST['page'])) include ('../survey_templates/$POST['[page'].php"); but that proved to be WAY too dangerous 😉

To add a bit more bulletproofing around weekender's suggestion,

if(in_array($_POST['option'], $options_array))
{   //Include the corresponding file
    include $path.$options_array[$_POST['option']];
}
else
{   //Handle a bogus option
}