2
\$\begingroup\$

I have this simple function here because I am too lazy to write out a state dropdown every time I need one. I already have an array with all the states set up, abbreviations as keys and names as values. It works just fine, but I like to write concise, clean, legible code, and I am still very unseasoned, so I find I miss a lot of stupidly simple s*** (the price I pay for teaching myself and not practicing often enough).

I see the four lines of redundant code inside the two primary conditionals, and I would move them outside the if statement, except that I have that last echo statement, which is really the whole purpose for the condition in the first place, and I don't think retesting the $style variable is all that kosher.

function create_state($style, $form = NULL) {
  global $state;
  echo '<select name="state">'."\n";
  echo '<option value="">--</option>'."\n";
  if ($style === 'abbr') {
    ksort($state);      
    foreach ($state as $abbr => $name) {
      echo "<option value=\"$abbr\"";
      if($form === $abbr)
        echo 'selected="selected"';
      echo ">$abbr</option>\n";
    }
  } else {
    asort($state);      
    foreach ($state as $abbr => $name) {
      echo "<option value=\"$abbr\"";
      if($form === $abbr)
        echo 'selected="selected"';
      echo ">$name</option>\n";
    }
  }
  echo '</select>'."\n";
}
\$\endgroup\$
0

1 Answer 1

Reset to default
1
\$\begingroup\$

I'm going to assume here (having done this myself before when I first started out with PHP), that you are storing the state data in a MySQL database, which you are pulling and storing in the global $state var.

Do not do that. You're slowing down the entire program by making a call out to the database for what is pretty much static data (the last state to join the union joined 55 years ago).

Now, I would just have the dropdowns stored in functions which just return a string. One function for long name, one for abbreviated.

However, using this as an exercise for you in DRY, I went ahead and created a class, StateDropDown, which contains an associative array of states/properties and their abbreviation. On initiation, the class generates the output form. If the $style contains 'abbr', then we use the abbreviated version after performing a ksort(). Otherwise, since the data in the array is static and already sorted as we prefer, we don't perform an asort and just go straight to storing the select form in a property $output. 

class StateDropDown{
    protected $states = array(
        "Alabama" => "AL",
        "Alaska" => "AK",
        "Arizona" => "AZ",
        "Arkansas" => "AR",
        "California" => "CA",
        "Colorado" => "CO",
        "Connecticut" => "CT",
        "Delaware" => "DE",
        "Florida" => "FL",
        "Georgia" => "GA",
        "Hawaii" => "HI",
        "Idaho" => "ID",
        "Illinois" => "IL",
        "Indiana" => "IN",
        "Iowa" => "IA",
        "Kansas" => "KS",
        "Kentucky" => "KY",
        "Louisiana" => "LA",
        "Maine" => "ME",
        "Maryland" => "MD",
        "Massachusetts" => "MA",
        "Michigan" => "MI",
        "Minnesota" => "MN",
        "Mississippi" => "MS",
        "Missouri" => "MO",
        "Montana" => "MT",
        "Nebraska" => "NE",
        "Nevada" => "NV",
        "New Hampshire" => "NH",
        "New Jersey" => "NJ",
        "New Mexico" => "NM",
        "New York" => "NY",
        "North Carolina" => "NC",
        "North Dakota" => "ND",
        "Ohio" => "OH",
        "Oklahoma" => "OK",
        "Oregon" => "OR",
        "Pennsylvania" => "PA",
        "Rhode Island" => "RI",
        "South Carolina" => "SC",
        "South Dakota" => "SD",
        "Tennessee" => "TN",
        "Texas" => "TX",
        "Utah" => "UT",
        "Vermont" => "VT",
        "Virginia" => "VA",
        "Washington" => "WA",
        "West Virginia" => "WV",
        "Wisconsin" => "WI",
        "Wyoming" => "WY",
        "American Samoa" => "AS",
        "District of Columbia" => "DC",
        "Federated States of Micronesia" => "FM",
        "Guam" => "GU",
        "Marshall Islands" => "MH",
        "Northern Mariana Islands" => "MP",
        "Palau" => "PW",
        "Puerto Rico" => "PR",
        "Virgin Islands" => "VI",
        "Armed Forces Africa" => "AE",
        "Armed Forces Americas" => "AA",
        "Armed Forces Canada" => "AE",
        "Armed Forces Europe" => "AE",
        "Armed Forces Middle East" => "AE",
        "Armed Forces Pacific" => "AP"
    );
    public $output = "";

    public function __construct($style = false, $form = null){
        if($style === true){
            ksort($this->states);
        }

        $this->output = "<select name='state'>";
        $this->output .= "<option value=''>--</option>";

        foreach($this->states as $name => $abbr){
            $this->output .= "<option value='" . $abbr . "'";
            if($form === $abbr){
                $this->output .= "selected='selected'";
            }

            if($style === true){
                $this->output .= ">" . $abbr . "</option>";
            } else {
                $this->output .= ">" . $name . "</option>";
            }
        }
        $this->output .= "</select>";
    }
}

To use, simply instantiate and echo output.

Here, we do a regular state name, with no selected state: 

$dropdown = new StateDropDown();
echo $dropdown->output;

Here, we do a regular state name, with a selected state, NY: 

$dropdown = new StateDropDown(null, "NY");
echo $dropdown->output;

Here, we do an abbreviated state name, with no selected state: 

$dropdown = new StateDropDown(true);
echo $dropdown->output;

Here, we do an abbreviated state name, with a selected state, FL: 

$dropdown = new StateDropDown(true, 'FL');
echo $dropdown->output;

While we may have the overhead of instantiating a new object, etc - we save on a possible database call. Additionally, we no longer do multiple checks, but rather do one check for abbreviation preference and assume the non-abbreviated by default, as well as keeping it non-abbreviated sort by default.

EDIT

Since OP indicated that I was incorrect in my assumption that they were storing this data in a database, I've modified my above Class into a function which does the same thing:

function statesDropdown($style = false, $form = null){
    $states = array(
        "Alabama" => "AL",
        "Alaska" => "AK",
        "Arizona" => "AZ",
        "Arkansas" => "AR",
        "California" => "CA",
        "Colorado" => "CO",
        "Connecticut" => "CT",
        "Delaware" => "DE",
        "Florida" => "FL",
        "Georgia" => "GA",
        "Hawaii" => "HI",
        "Idaho" => "ID",
        "Illinois" => "IL",
        "Indiana" => "IN",
        "Iowa" => "IA",
        "Kansas" => "KS",
        "Kentucky" => "KY",
        "Louisiana" => "LA",
        "Maine" => "ME",
        "Maryland" => "MD",
        "Massachusetts" => "MA",
        "Michigan" => "MI",
        "Minnesota" => "MN",
        "Mississippi" => "MS",
        "Missouri" => "MO",
        "Montana" => "MT",
        "Nebraska" => "NE",
        "Nevada" => "NV",
        "New Hampshire" => "NH",
        "New Jersey" => "NJ",
        "New Mexico" => "NM",
        "New York" => "NY",
        "North Carolina" => "NC",
        "North Dakota" => "ND",
        "Ohio" => "OH",
        "Oklahoma" => "OK",
        "Oregon" => "OR",
        "Pennsylvania" => "PA",
        "Rhode Island" => "RI",
        "South Carolina" => "SC",
        "South Dakota" => "SD",
        "Tennessee" => "TN",
        "Texas" => "TX",
        "Utah" => "UT",
        "Vermont" => "VT",
        "Virginia" => "VA",
        "Washington" => "WA",
        "West Virginia" => "WV",
        "Wisconsin" => "WI",
        "Wyoming" => "WY",
        "American Samoa" => "AS",
        "District of Columbia" => "DC",
        "Federated States of Micronesia" => "FM",
        "Guam" => "GU",
        "Marshall Islands" => "MH",
        "Northern Mariana Islands" => "MP",
        "Palau" => "PW",
        "Puerto Rico" => "PR",
        "Virgin Islands" => "VI",
        "Armed Forces Africa" => "AE",
        "Armed Forces Americas" => "AA",
        "Armed Forces Canada" => "AE",
        "Armed Forces Europe" => "AE",
        "Armed Forces Middle East" => "AE",
        "Armed Forces Pacific" => "AP"
    );
    $output = "";
    if($style === true){
        ksort($states);
    }

    $output = "<select name='state'>";
    $output .= "<option value=''>--</option>";

    foreach($states as $name => $abbr){
        $output .= "<option value='" . $abbr . "'";
        if($form === $abbr){
            $output .= "selected='selected'";
        }

        if($style === true){
            $output .= ">" . $abbr . "</option>";
        } else {
            $output .= ">" . $name . "</option>";
        }
    }
    $output .= "</select>";

    return $output;
}

To output the dropdown, simply do:

echo statesDropdown();
\$\endgroup\$
2
  • \$\begingroup\$ Well, the good news is I had never intended to store such data in a database, because as you said, it almost never changes, unless I need to add US territories or Canadian provinces, at which point I can simply amend my array. The bad news is I have no experience with classes and little understanding of exactly how they work, so I will have to look at your code above until I understand it (I don't like using what I can't understand). I did get some feedback on the initial question on SO, which did simplify the original function, but I will update when I have decided which route to take. \$\endgroup\$ Commented Sep 7, 2014 at 21:13
  • \$\begingroup\$ Totally understand - though I would suggest taking the class above as a learning exercise as they play an integral role in programming. I've also gone ahead and modified my answer so that it does not use a class. \$\endgroup\$ Commented Sep 8, 2014 at 14:20

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.