PHP if-then-else statement not working.

Question

My url is something such as: "inventory.php?sorting=1" and so forth. Page loads fine but does not display the information properly.

mysql_connect("localhost","user","pass"); 
mysql_select_db("database"); 

if ($sorting == 1){
$result = mysql_query("select * from vehicles ORDER BY year DSC");
}
elseif ($sorting == 2){
$result = mysql_query("select * from vehicles ORDER BY make DSC");
}
elseif ($sorting == 3){
$result = mysql_query("select * from vehicles ORDER BY miles DSC");
}
elseif ($sorting == 4){
$result = mysql_query("select * from vehicles ORDER BY downpay DSC");
}
elseif ($sorting == 5){
$result = mysql_query("select * from vehicles ORDER BY pricepay DSC");
}
elseif ($sorting == 6){
$result = mysql_query("select * from vehicles ORDER BY pricecash DSC");
}
else {
$result = mysql_query("select * from vehicles");
}

while($r=mysql_fetch_array($result))

Answer 1

Short answer: Replace $sorting with $_GET["sorting"], or add $sorting = $_GET['sorting']; to the top of your code.

Long answer: A long time ago, register_globals was used to automatically make URL parameters appear as variables. This lead to a lot of security problems (the above link contains an example), so it was eventually turned off by default (PHP 4.2.0). In PHP 6, this option no longer exists. Thus, you need to explicitly access URL GET parameters through $_GET or $_REQUEST.

As an alternative, you can explicitly import your URL parameters into local variables by using the import_request_variables command.

Answer 2

You need to replace $sorting with $_GET["sorting"]

but, also:

Would it not be a better idea to use the switch statement?

switch($_GET["sorting"]{
    case 1:
 $result = mysql_query("select * from vehicles ORDER BY year DSC");
 break;
case 2:

etc.

Answer 3

Is there some

$sorting = $_GET['sorting'];

somewhere in your code? It won't get it's value automatically.

Answer 4

Add this line at the start of your code.

$sorting = $_REQUEST['sorting'];

Answer 5

Why not use switch:

switch ($sorting) {
    case 1:
        $result = mysql_query("select * from vehicles ORDER BY year DSC");
        break;
    case 2:
        $result = mysql_query("select * from vehicles ORDER BY make DSC");
        break;
    // ...
    default:
        $result = mysql_query("select * from vehicles");
        break;
}

Also, make sure $sorting is assigned:

$sorting = $_GET['sorting']; // Place somewhere before the switch

Answer 6

You need to get the $sorting variable from the $_GET array. I would also rewrite it as a switch statement like this:

switch($_GET['sorting'])
{
  case 1:
    $result = mysql_query("select * from vehicles ORDER BY year DSC");
  brek;

  case 2:
    $result = mysql_query("select * from vehicles ORDER BY make DSC");
  break;

  ...

  default:
    $result = mysql_query("select * from vehicles");
  break;
}

Answer 7

Why not just use the field name as the GET variable?

$sortField = $_GET['sorting'];
// Ensure we don't get any SQL injection:
$validFields = array('year', 'make', 'miles' ... 'pricecash');


$sql = "select * from vehicles";

if(in_array($sortField, $validFields)){
    $sql .= ' ORDER BY ' . $sortField .' DESC';
}

mysql_query($sql);

and then access the page using inventory.php?sorting=year etc.

This makes the URL more readable, predicatable and means you can support new fields by just adding them to the array without needing to write new switch cases.

Answer 8

And to make it nicer, you can do this:

$sortBy = '';
switch($_GET["sorting"]{
  case 1:
    $sortBy = 'year';
    break;
  case 2:
    $sortBy = 'make';
    break;
  //...
}  

if(!empty($sortBy)) {
  $result = mysql_query('select * from vehicles ORDER BY ' . $sortBy . ' DSC');
}
else {
  $result = mysql_query('select * from vehicles');
}

This way, you only have to change your query at one point if you have to change it someday.

Answer 9

you can use $_GET['sorting'] or $_REQUEST['sorting'] if it could come in by either get or post, but why not do this?

$query = "SELECT * FROM `vehicles`";

$sort_values = array( 1 => 'year', 'make', 'miles', 'downpay', 'pricepay', 'pricecash' );
$sort_number = $_GET['sorting'];
if( $sort_number <= count($sort_values) ) {
    $query .= " ORDER BY `{$sort_values[ $sort_number ]}` DESC";
}

$result = mysql_query($query);

note that the 1 => portion of the array is because you 1-indexed your list of queries.
the reason for the <= portion of the if statement is for that reason too -- if you 0-indexed it, you would just use <.

It may not seem like it yet, but you'll quickly find out that it's worth it to try and find ways to write less code. Using the array means you don't have to copy / paste any code (repeatedly writing $result = mysql_query(...);, etc) and it is virtually effortless to add new columns to your table, should you ever need to display more information.

One might even fetch the column names from the database directly and avoid ever touching this code again.