Dropdown menus question

The following code works as intended, but could it be shortened? Everything is placed into a variable named $page
and sent to another function to be printed. I know just because something works, doesn’t mean it is efficient.
Please don’t be too hateful to me should you see something terribly bad.


$year  = !isset($_POST['year'])  ? isset($_GET['year'])  ? $_GET['year']  : '2011' : $_POST['year'];
$state = !isset($_POST['state']) ? isset($_GET['state']) ? $_GET['state'] : 'KS'   : $_POST['state'];
$sport = !isset($_POST['sport']) ? isset($_GET['sport']) ? $_GET['sport'] : 1      : $_POST['sport'];

// A form that shows three dropdown boxes across the top of the screen for the user to select from
$page = <<<END
<form action="admin.php?do=teams" method="post">
  <table>
    <tr>
      <td>Year: 
END;

  // Shows SELECTED year to user in dropdown box
  $page .= "<select name=\\"year\\">\
<option selected value=\\"".$year."\\">".$year."</option>\
";

  // Searches table xx_teams for all years posted. If there is only one record and it was
  // made in 2011, then 2011 would be the only year it would find. More records with the same year
  // would be omitted because of the DISTINCT in the query.
  $query = "SELECT DISTINCT " .
           "year " .
           "FROM {{table}} " .
           "ORDER BY year ASC";
  $result = doquery($query, "teams");

  // Dropdown box with all the years available for selection
  while ($row = mysql_fetch_array($result)) { $page .= "<option value=\\"".$row['year']."\\">".$row['year']."</option>\
"; }

$page .= <<<END
        </select>
      </td>
      <td>State: 
END;

  // Shows SELECTED state to user in dropdown box
  $page .= "<select name=\\"state\\">\
<option selected value=\\"".$state."\\">".$state."</option>\
";

  // Searches table xx_teams for all states posted just as the above code did for years.
  // States are not saved as an id number. I chose to save them as two letter abbreviations.
  $query = "SELECT DISTINCT " .
           "state " .
           "FROM {{table}} " .
           "ORDER BY state ASC";
  $result = doquery($query, "teams");

  // Dropdown box with all the states ( abbreviations ) available for selection
  while ($row = mysql_fetch_array($result)) { $page .= "<option value=\\"".$row['state']."\\">".$row['state']."</option>\
"; }

$page .= <<<END
      </select>
      </td>
      <td>Sport: 
END;

  // This just puts the sport id into SELECTED numerical value
  $page .= "<select name=\\"sport\\">\
<option selected value=\\"".$sport."\\">";

  // This searches table xx_sport for the sport associated with the value above.
  // I did it this way because sport is a number in each record that has little meaning to user
  // It takes the number, matches it with the id and gets the sport
  $query = "SELECT DISTINCT " .
           "ws_sport.sport, ws_sport.id " .
           "FROM {{table}} " .
           "WHERE ws_sport.id='$sport' LIMIT 1";
  $result = doquery($query, "sport");

  $row = mysql_fetch_array($result);

  // Show selected sport
  $page .= $row['sport']."</option>\
";

  // Search table xx_sport to fill dropdown box with all available sports
  $query = "SELECT DISTINCT " .
           "ws_sport.sport, ws_sport.id " .
           "FROM {{table}} " .
           "ORDER BY ws_sport.id ASC";
  $result = doquery($query, "sport");

  // Dropdown box with all sports listed
  while ($row = mysql_fetch_array($result)) { $page .= "<option value=\\"".$row['id']."\\">".$row['sport']."</option>\
"; }

$page .= <<<END
      </select>
      </td>
      <td class="data"><input type="submit" name="submit" value="Submit" /></td>
    </tr>
  </table>
</form>
END;

Personally I don’t like this:

$year  = !isset($_POST['year'])  ? isset($_GET['year'])  ? $_GET['year']  : '2011' : $_POST['year'];

You use 2 x “? :” in one line, it is really hard to read such code, it would be better to rewrite it to use just 1 x “? :” or to rewrite it using “if, elseif, else”. For example like this:

// Alternative 1
// Takes just from $_POST or from $_GET
$year = null;
if (array_key_exists('year', $_POST) && 1 * $_POST['year'])
    $year = $_POST['year'];
elseif(array_key_exists('year', $_GET) && 1 * $_GET['year'])
    $year = $_GET['year'];
else
    $year = date('Y');

// Alternative 2
// Takes from $_POST or from $_GET or from $_COOKIE, you could 
// avoid taking from $_COOKIE by putting this line of code unset($_COOKIE['year']);
$year = null;
if (array_key_exists('year', $_REQUEST) && 1 * $_REQUEST['year'])
    $year = $_REQUEST['year'];
else 
    $year = date('Y');

// alternative 3
// Uses just 1 x "? :"
$year = null;
$year = (array_key_exists('year', $_REQUEST)) ? $year = $_REQUEST['year'] : $year = date('Y');

echo $year;

The next thing I noticed is that you are using Heredoc syntax at wrong places.

$page .= <<<END
      </select>
      </td>
      <td>Sport: 
END;

This could be rewritten like this:

$page .= '</select></td><td>Sport:';

Read up more about Heredoc here http://php.net/manual/en/language.types.string.php.

yes, you could simplify that code by creating a selectbox function, take a read of this post http://www.sitepoint.com/forums/showthread.php?753900-Page-layout-recomendation-please&highlight=selectboxmatch



function doquery($query, $table) {
    include 'config.php';
    $sqlquery = mysql_query(str_replace("{{table}}", $dbsettings["prefix"] . "_" . $table, $query)) or die(mysql_error());
    return $sqlquery;
}

function showOptionsDrop($page, $array, $active){

        foreach($array as $k => $v){
            $s = ($active == $v)? ' selected="selected"' : '';
            $page .= '<option value="'.$v.'"'.$s.'>'.$v.'</option>'."\
";
        }

        return $page;
}

  $query = "SELECT DISTINCT " .
           "year " .
           "FROM {{table}} " .
           "ORDER BY year ASC";  $result = doquery($query, "teams");

  $result = doquery($query, "teams");

  $page .= "<select name=\\"year\\">\
";

  showOptionsDrop($page, $result, $year);

$page .= <<<END
        </select>
      </td>
      <td>State: 
END;


Why does this not print out my Dropbox full of the options within $result?
It should append $page within the function to $page in the main page.

I changed my function to this:


$name = 'year';
$year = 2011;
$row = mysql_fetch_array($result);

$page .= dropdown ( $name, $row, $year );

function dropdown( $name, $options, $selected )
{
    $dropdown = '<select name="'.$name.'">'."\
";
    foreach( $options as $option )
    {
        $select = $selected==$option ? ' selected' : null;
        $dropdown .= '<option value="'.$option.'"'.$select.'>'.$option.'</option>'."\
";
    }
    $dropdown .= '</select>'."\
";
    return $dropdown;
}

$row = array ( 2011, 2010, 2009 ) // these are the values in it but FOREACH only shows 2009 in the dropdown box once it is made.
Any help would be appreciated.

Your PHP code may well be working as you expect but when you look at the source code, you realise that you have created incorrect html markup … its an easy trap to fall into.

What jumps out at me first though is that you are making a weak comparison which can often cause you heartbreak.

You have your code assessing these two variables, you are only checking the values match.


$selected==$option

You could be checking for equality with ===.

Do a temporary var_dump() of each of those values and check that they match what you think they are holding, strings, integers etc.

http://www.php.net/manual/en/types.comparisons.php

This line looks mixed up to me:


$dropdown .= '<option value="'.$option.'"'.$select.'>'.$option.'</option>'."\
";

… which might be causing the first problem I mentioned…

try:


$dropdown .= "<option value='$option' $select>$option</option>". PHP_EOL ;

You are selecting some years from your database which may be unavoidable and you have due cause to do this, but did you know you can do this to generate an array of year numbers?


$years = range(date('Y'), date('Y') - 2);

var_dump($years);

//array
//  0 => int 2011
//  1 => int 2010
//  2 => int 2009

EDIT;

Anyhow, this works as expected:


$years = range(date('Y'), date('Y') - 2);
$name = 'year';
$year = 2011;
$page = "";

function dropdown( $name, $options, $selected )
{
    $dropdown = "<select name='$name'>" . PHP_EOL;
    foreach( $options as $option )
    {
        $select = $selected==$option ? ' selected' : '';
        $dropdown .= "<option value='$option' $select>$option</option>" . PHP_EOL;
    }
    $dropdown .= "</select>";
    return $dropdown;
}

$page .= dropdown( $name, $years, $year );
echo $page;

** PHP_EOL is a PHP CONST which contains a the OS dependent line end, instead of messing with
and quotes and so on…