[SOLVED] Code Cleaning... Giving me headaches

[monkeypaw201]

Can someone please help me clean up this code? I wrote it yet it still gives me headaches everytime i look at it

 

<?php
if(isset($_SESSION['pilot_id']))
{
   $pilot = mysql_query("SELECT * FROM `users` WHERE `pilot_id` = '$_SESSION[pilot_id]'");

   $row_pilot = mysql_fetch_array($pilot);
   
if($row_aircraft['rank'] == "Regional" && ($row_pilot['rank'] == "Regional" || $row_pilot['rank'] == "Short Haul" || $row_pilot['rank'] == "Medium Haul" || $row_pilot['rank'] == "Long Haul"))
{
$file = "file";
$rankreq = "1";
}
elseif($row_aircraft['rank'] == "Short Haul" && ($row_pilot['rank'] == "Short Haul" || $row_pilot['rank'] == "Medium Haul" || $row_pilot['rank'] == "Long Haul"))
{
$file = "file";
$rankreq = "1";
}
elseif($row_aircraft['rank'] == "Medium Haul" && ($row_pilot['rank'] == "Medium Haul" || $row_pilot['rank'] == "Long Haul"))
{
$file = "file";
$rankreq = "1";
}
elseif($row_aircraft['rank'] == "Medium Haul" && $row_pilot['rank'] == "Long Haul")
{
$file = "file";
$rankreq = "1";
}else{
$file = "details";
$rankreq = "0";
}

if($row_route['category'] == "International")
{
   if($row_pilot['international'] == "Yes")
   {
   $file = "file";
   }else{
   $file = "details";
   $rankintl = "0";
   }
}else{
$file = "details";
}

}
else
{
$file = "details";
}
?>

            <td>  <?php
            		if($file == "file")
            		{
            		echo "<a href='file.php?id=" . $row_timetable['id'] . "' ";
            		?>
            		onclick="javascript:return confirm('Are you sure you want to place a bid on this flight?')"
            		<?php
            		echo " ><img src='http://www.cormorantair.com/images/pirep.jpg' hieght='15' width='15'>";
            		}
            		else
            		{
            		echo "<a href='details.php?id=" . $_GET['id'] . "' ";
            		?>
            		onclick="alert('Sorry, It looks like you are not meeting all the requirements to fly this route. The missing requirements are listed below. \n\n <?php if(!isset($_SESSION['pilot_id'])){echo "- You Must Be Logged In";}else{ ?><?php if(!isset($rankreq)){echo "- You Do Not Have A High Enough Rank \n";} ?><?php if(!isset($rankintl)){echo "- You Are Not Certified For International Flights \n";}} ?>')"
            		<?php
            		echo " ><img src='http://www.cormorantair.com/images/pirep.jpg' hieght='15' width='15'>";
            		}
            		?>

 

thx

[btherl]

Do you have some kind of ordering on "Regional", "Long Haul", "Medium Haul" and "Short Haul"?  If so make an associative array like this:

 

$rank_order = array(
  'Regional' => 0,
  'Long Haul' => 1,
  'Medium Haul' => 2,
  'Short Haul' => 3,
);

 

Then your comparisons are just numerical comparisons instead of listing out all the cases.  Does that fit or do I misunderstand what your code is doing?

[Psycho]

Well, first off the third elseif is unnecessary since that condition wouold be covered in the else if right before it. I *think* maybe the first part of that clause was to be Long Haul???

 

I also see another problem that I'm not sure what you meant to do. Int he long line of if/else's you set a value for the variable $file. But then before you use that variable you have another if/else for international in which you always reset the value of $file. So all the preceeding logic is wasted.

 

You didn't explain any of the logic behind the code, but from looking at it I am assuming that there are four different values for aircraft and pilot (Regional, Short Haul, Medium Haul & Long haul). Also, if the pilot's rank is >= the aircraft rank then you set $file = "file"; and  $rankreq = "1";. based upon that there is a much easier method.

 

There are probably other changes I would make, but again I don't know all the issues around the logic:

 

<?php

if(isset($_SESSION['pilot_id']))
{
   $pilot = mysql_query("SELECT * FROM `users` WHERE `pilot_id` = '$_SESSION[pilot_id]'");
   $row_pilot = mysql_fetch_array($pilot);

   $ranks = array('Regional', 'Short Haul', 'Medium Haul', 'Long Haul');

   if (array_search($row_pilot['rank'], $ranks) >= array_search($row_aircraft['rank'], $ranks))
   {
       $file = "file";
       $rankreq = "1";
   }
   else
   {
       $file = "details";
       $rankreq = "0";
   }

   if($row_route['category'] == "International")
   {
       if($row_pilot['international'] == "Yes")
       {
           $file = "file";
       }else{
           $file = "details";
           $rankintl = "0";
       }
   }
// This logic always overwrites the first set of conditions ???
//    else
//    {
//        $file = "details";
//    }

}
else
{
   $file = "details";
}

?>

 

I would probably create an array with 0=>'details' and 1=>'file' instead of setting two values as they always seem to be in tandem.

[monkeypaw201]

Here's the logic...

 

First off, all of this is to decide what a link directs to.

 

There are 4 Ranks:

• Regional
  
• Short Haul
  
• Medium Haul
  
• Long Haul
  

 

And international certification

 

And 6 aircraft. Each aircraft has a minimum rank associated to it. (ie Regional is limited to Aircraft A and Aircraft B, and SHort Haul has Aircraft C, D AND the aircraft from Regional cause its a higher rank)

 

The purpose is to check the pilot's rank and see if they are certified to fly the flight on that page. If they are the link goes to file.php if they aren't it links to details.php and adds what they are missing to the javascript alert popup..

 

Hope this helps... Does the code you (mjdamato) put work for this logic? Looking at it im not so sure..

 

Thanks

[monkeypaw201]

bump

[Psycho]

Well, you still didn't account for the problem regarding the broken logic around the international else clause - which I commented out in the code I posted.

 

The code I posted that replaces much of your previous logic does exactly what you described. If the pilot's rank is greater than or equal to the aircraft rank, then they will link to 'file', if not they will link to 'details'.

 

Anyway, based upon my understanding of your problem this is how I would do it (I'm sure there are some minor typos). There may be some slight logic errors based upon misunderstanding of the logic needed - which wouold just need some minor corrections. But, this presents a much cleaner implementation of what you need. I think it is always best to separate the logic from the presentation (i.e. don't put a lot of IF/ELSE statements within echo's.

 

<?php

if(isset($_SESSION['pilot_id']))
{
   $pilot = mysql_query("SELECT * FROM `users` WHERE `pilot_id` = '$_SESSION[pilot_id]'");
   $row_pilot = mysql_fetch_array($pilot);

   $ranks = array('Regional', 'Short Haul', 'Medium Haul', 'Long Haul');


   $pilot_rank    = array_search($row_pilot['rank'], $ranks);
   $aircraft_rank = array_search($row_aircraft['rank'], $ranks);

   //Set qualified based upon aircraft/pilot match
   $qualified = ($pilot_rank >= $aircraft_rank)?true:'You Do Not Have A High Enough Rank';

   //Check add'l qualification if intn'l
   if($row_route['category'] == "International")
   {
       $qualified = ($row_pilot['international']=="Yes")?true:'You Are Not Certified For International Flights';
       $intl = true;
   }
}
else
{
   $qualified = 'You Must Be Logged In';
}

//Create variables for href and onlclick content
if($qualified===true)
{
   $href = "file.php?id={$row_timetable['id']}";
   $onclick = "javascript:return confirm('Are you sure you want to place a bid on this flight?');";
}
else
{
   $href = "details.php?id={$_GET['id']}";
   $onclick  = "alert('";
   $onclick .= "Sorry, It looks like you are not meeting all the requirements to fly this route.\n";
   $onclick .= "The missing requirements are listed below. \n\n- {$qualified}\n";
   $onclick .= "');";
}

echo "<td>\n";
echo "<a href=\"$href\" onclick=\"{$onclick}\">";
echo "<img src=\"http://www.cormorantair.com/images/pirep.jpg\" hieght=\"15\" width=\"15\">";
echo "</a>\n";
echo "</td>\n";

?>

 

Oh yeah, you should really add some validation around any user input (e.g. GET['id'], and $_SESSION[pilot_id] if you don't validate before saving it)

[monkeypaw201]

Works perfectly except the javascript alert doesn't work..

[Psycho]

As I said, I'm sure there are typos. Did you look at the source generated to see what might be wrong with the alert code?

 

The problem was due to the \n's being interpteted in the PHP code and not being available in the javascript. There are two simple fixes. 1) Change the strings with \n's to be created with single-quotes, so the won't be interpreted, or 2) use \\n in the PHP strings and they will be created as \n in the Javavascript.

 

This code will fix the issue:

    $onclick  = "alert('";
    $onclick .= 'Sorry, It looks like you are not meeting all the requirements to fly this route.';
    $onclick .= 'The missing requirements are listed below. \n\n- ' . $qualified . '\n';
    $onclick .= "');";

[monkeypaw201]

NVM... It works, thank you so much!