1

I'm working on a type of directory. One Select section has a place where a user can choose their state. The next select section contains 4 values for listings.

When they choose their appropriate state and the listing type it's supposed to pull back listings in that state.

Unfortunately, I'm having 2 problems.

  1. The first 4 listing types work beautifully. If you choose California and one of the first 2 listing types all of the data comes back. But if you choose a state and either listing type 3 - 4, it throws the error I created "Please choose a state and a listing type". This doesn't make sense, because the code for the 3rd and 4th listing types are identical to the 1st 2.

  2. I'm using switch commands to trigger a class >> function for each state. I've provided a condensed version of what this looks like, but I have like 600+ lines of switch/case statements and that seems excessive. But don't it with if/elseif commands would be even more prolific.

HTML

<form action="" method="post">
        <select name="state">
            <option>Choose State</option>           
            <option value="AL">AL</option>
            <option value="AK">AK</option>
            <option value="AZ">AZ</option>
            <option value="AR">AR</option>
        </select>

        <select name="type">
            <option>Type</option>
            <option value="location">Location</option>  
            <option value="hotels">Hotels</option>              
            <option value="cars">Cents</option>     
            <option value="trucks">Trucks</option>      
        </select>
        <p><input type="submit" name="submit" value="Search For" /></p>
    </form>

PHP

if($_SERVER['REQUEST_METHOD'] == 'POST') {

$state              = $_POST['state']; 
$accommodation_type = $_POST['accommodation-type'];             

if( isset($state) && (isset($accommodation_type)) ) {       


            if($accommodation_type == 'location') {
                require('includes/location.php');
                    switch($state) {
                        case $state == 'AL':
                            $location = new Location();
                            $location->Alabama_Location();
                            break;                                 
                        case $state == 'AK':
                            $location = new Location();
                            $location->Alaska_Location();
                            break;                                     
                        case $state == 'AZ':
                            $location = new Location();
                            $location->Arizona_Location();
                break;                                                 
                    }   
                }


            elseif($accommodation_type == 'hotels') {
                require('includes/hotels.php');
                switch($state) {
                    case $state == 'AL':
                        $hotel = new Hotels();
                        $hotel->Alabama_Hotels();
                        break;                                     
                    case $state == 'AK':
                        $hotel = new Hotels();
                        $hotel->Alaska_Hotels();
                        break;                                     
                    case $state == 'AZ':
                        $hotel = new Hotels();
                        $hotel->Arizona_Hotels();
                 break;                                                
                }   

            } else { 
                echo "Please choose a state and an accommodation type"; 
            }


} else { 

} } else {}

So I need a way to simplify this, because I've condensed this and when you tally up 50 states x 4 categories you get 200 case statements. Seems excessive.

Also I still don't understand why it loads the right data for the first 2 options, but not the last 2, when all I did was copy the working code and repeat it with different data.

Plus, it's not checking to see if both state and type are selected. It only shows the user the warning when state hasn't been selected.

Any thoughts? and thank you for any help.

  • Why do you have an object for each state for each category? Why not `$hotel = new Hotels($state);`, then `$hotel->Hotels();`? Or just modify it to, `$hotel->Hotels($state);`. And your `if` statement lacks a selector for `"cars"` and `"trucks"`. – Der Kommissar Apr 29 '15 at 18:50
  • i think instead of `$_POST['accommodation-type']` u need to get `$_POST['type']` in ur post – Abdul Rehman Apr 29 '15 at 18:54

1 Answers1

1

Your switch statement has a wrong syntax. By using case $state == 'AL': you essentially define case true: / case false:. Only leave the values themselves there.

switch($state) {
    case 'AL':
        $location = new Location();
        $location->Alabama_Location();
        break;                                 
    case 'AK':
        $location = new Location();
        $location->Alaska_Location();
        break;                                     
    case 'AZ':
        $location = new Location();
        $location->Arizona_Location();
        break;                                                 
}

Also, you're not sending any form fields with the name attribute set to accommodation-type, and thus you'll get null and an E_NOTICE level error when you access it using $accommodation_type = $_POST['accommodation-type'];, use the field you do have instead, which is only called type.

I also suggest you take a look at How do I make a placeholder for a 'select' box?. Plus, require is a keyword, not a function, so you don't need the parentheses.

Finally, for your many switch cases, I suggest the following: Make an associative array like this:

$statemap = array(
    'AL' => 'Alabama',
    'AK' => 'Alaska',
    'AZ' => 'Arizona',
);

Then, call the function for the state that the user selected:

if (isset($statemap[$state])){
    $location = new Location();
    $func = $statemap[$state].'_Location';
    $location->{$func}();
}

This will save you a couple hundred cases. Even more dynamic, a function that can handle both (and possibly more) of your switch statements with the appropriate associative array(s) defined:

function shortSwitch($map, $key, $type){
    if (isset($map[$key])){
        $class = new $type();
        $func = "{$map[$key]}_$type";
        $class->{$func}();
    }
}

Call it like this:

shortSwitch($statemap, $state, 'Location');
shortSwitch($statemap, $state, 'Hotels');
Community
  • 1
  • 1
SeinopSys
  • 8,787
  • 10
  • 62
  • 110
  • Thank you! I'll give this a shot! Yes..how silly how I was using those case statements. I should have known better. Thank you for the suggestions for simplifying it as well. The problem is Type is a class and each function within the class represents a state with unique data to that state. I don't know any other way to do this. So I have 4 classes with 50 functions each that each call upon a specific state page that echoes out the listings. – user1370182 Apr 29 '15 at 22:32