What's wrong with this code?

PHP programming forum. Ask questions or help people concerning PHP code. Don't understand a function? Need help implementing a class? Don't understand a class? Here is where to ask. Remember to do your homework!

Moderator: General Moderators

Post Reply
Kofikoduah
Forum Newbie
Posts: 8
Joined: Tue Mar 21, 2006 7:45 am

What's wrong with this code?

Post by Kofikoduah »

Can someone help me with the logic of this code especially the ifs what i want to achieve is for the script to select an .inc file (which holds a function) based on what the corresponding $_post[] is. Are my else's in place or are they not necessary

Code: Select all

<?php
require_once('../../Connections/mysql.php');

//$all[] = $_POST["all"];
$all[] = $_POST["greater_accra"];
$all[] = $_POST["ashanti_region"];
$all[] = $_POST["central_region"];
$all[] = $_POST["western_region"];
$all[] = $_POST["eastern_region"];
$all[] = $_POST["volta_region"];
$all[] = $_POST["brong_ahafo"];
$all[] = $_POST["northern_region"];
$all[] = $_POST["upper_west"];
$all[] = $_POST["upper_east"];

//if ($all[] = $_POST["all"]) {generate_all();}
if  ($all[] == $_POST["greater_accra"]) {require_once('by_region/genccadv_GH_gar.inc');}
else if  ($all[] == $_POST["ashanti_region"]) {require_once('by_region/genccadv_GH_ashR.inc');}
else if  ($all[] == $_POST["central_region"]) {require_once('by_region/genccadv_GH_centR.inc');}
else if  ($all[] == $_POST["western_region"]) {require_once('by_region/genccadv_GH_westR.inc');}
else if  ($all[] == $_POST["eastern_region"]) {require_once('by_region/genccadv_GH_eastR.inc');}
else if  ($all[] == $_POST["volta_region"]) {require_once('by_region/genccadv_GH_voltaR.inc');}
else if  ($all[] == $_POST["brong_ahafo"]) {require_once('by_region/enccadv_GH_brongR.inc');}
else if  ($all[] == $_POST["northern_region"]) {require_once('by_region/genccadv_GH_northR.inc');}
else if  ($all[] == $_POST["upper_west"]) {require_once('by_region/genccadv_GH_uwestR.inc');}
else if  ($all[] == $_POST["upper_east"]) {require_once('by_region/genccadv_GH_ueastR.inc');}

//if ($all[] = $_POST["all"]) {generate_all();}
if  ($all[] == $_POST["greater_accra"]) {generate_Accra();}
else if  ($all[] == $_POST["ashanti_region"]) {generate_Ashanti();}
else if  ($all[] == $_POST["central_region"]) {generate_CentralCoast();}
else if  ($all[] == $_POST["western_region"]) {generate_Western();}
else if  ($all[] == $_POST["eastern_region"]) {generate_Eastern();}
else if  ($all[] == $_POST["volta_region"]) {generate_Volta();}
else if  ($all[] == $_POST["brong_ahafo"]) {generate_BrongAhafo();}
else if  ($all[] == $_POST["northern_region"]) {generate_Northern();}
else if  ($all[] == $_POST["upper_west"]) {generate_UpperWest();}
else if  ($all[] == $_POST["upper_east"]) {generate_UpperEast();}

?>
User avatar
Maugrim_The_Reaper
DevNet Master
Posts: 2704
Joined: Tue Nov 02, 2004 5:43 am
Location: Ireland

Post by Maugrim_The_Reaper »

A switch statement is usually both simpler and faster than lots of if...elseif comparisons. Your use of $all[] I'm not sure of - are you adding all the POST values to an array? And if so how are you reference any one value?

Try making these all option boxes in your form - select one with name="region" to create a key $_POST['region'] you can compare to your allowed list of values. You can also add a default for non-matching values. If region was sent by URL you can use $_GET['region'] from URL like index.php?region=greater_accra.

Code: Select all

switch($_POST['region'])
{
	case 'greater_accra':
		require_once 'by_region/genccadv_GH_gar.inc';
		generate_Accra();
		break;
	case 'ashanti_region':
		require_once 'by_region/genccadv_GH_ashR.inc';
		generate_Ashanti();
		break;
	case 'central_region':
		require_once 'by_region/genccadv_GH_centR.inc';
		generate_CentralCoast();
		break;
	case 'western_region':
		require_once 'by_region/genccadv_GH_westR.inc';
		generate_Western();
		break;
	case 'eastern_region':
		require_once 'by_region/genccadv_GH_eastR.inc';
		generate_Eastern();
		break;
	case 'volta_region':
		require_once 'by_region/genccadv_GH_voltaR.inc';
		generate_Volta();
		break;
	case 'brong_ahafo':
		require_once 'by_region/enccadv_GH_brongR.inc';
		generate_BrongAhafo();
		break;
	case 'northern_region':
		require_once 'by_region/genccadv_GH_northR.inc';
		generate_Northern();
		break;
	case 'upper_west':
		require_once 'by_region/genccadv_GH_uwestR.inc';
		generate_UpperWest();
		break;
	case 'upper_east':
		require_once 'by_region/genccadv_GH_ueastR.inc';
		generate_UpperEast();
		break;
	default:
		require_once 'by_region/genccadv_GH_gar.inc'; // default kicks in if no match found
		generate_Accra();
}
Few things.

$_POST and other array keys use single-quotes - $_POST['key'] not $_POST["key"]. $_POST is itself an array of values - no need for $all[] = x type operations. You just need a specific parameter to grab from $_POST, in the case above I used $_POST['region'] which is a value set via a form.

Now if you were allowing multiple values which I think you are doing - then just check if $_POST['greater_accra'] is a valid value - maybe its being set to 1 in your form using checkboxes? The HTML for this check would tell us a lot more.
Kofikoduah
Forum Newbie
Posts: 8
Joined: Tue Mar 21, 2006 7:45 am

Thank's so much

Post by Kofikoduah »

Thank you so much for the revision as well as the tips they are invaluable. The code just worked as soon as i put your lines in. Kudos! :D
Post Reply