Hi,

I am a newbie PHP programmer, I have some code that works but I want some tips on how 
I an Improve my code, i.e. should I be doing my updates / deletes on same php page as 
the display page, am I using transactions correctly, am I capturing SQL errors 
correctly am I handling form data as efficient as possible?

My code displays some information from a database and gives users the chance to delete 
or edit any field and is as follows: 

<? 

include ("../db.php");

$acton = $_POST['action'];

if ($action == "update") {
  if (isset($_POST['delete'])) {
    $deleteList = join(', ', $_POST['delete']);
  }

  //Enter info into the database
  mysql_query("begin");
  foreach ($_POST['fleet_id'] as $key => $value) {
    $fleetCode = $_POST['fleet_code'][$key];
    $historyUrl = $_POST['history_url'][$key];
    $downloadUrl = $_POST['download_url'][$key];
    mysql_query("UPDATE imp_fleet SET fleet_code = '$fleetCode', history_url = 
'$historyUrl', download_url = '$downloadUrl' WHERE fleet_id = $value") or die 
(mysql_error());
  }
  if ($deleteList) {
    mysql_query("DELETE FROM imp_fleet WHERE fleet_id IN($deleteList)") or die 
(mysql_error());
  }
  if (mysql_error()) {
    echo ("There has been an error with your edit / delete request. Please contact the 
webmaster");
    mysql_query("rollback");
  } else {
    mysql_query("commit");
  }
}

?>
<html>
<head>
  <title></title>
</head>
<body>
<form name="edit" method="post">
<h1>Edit / Delete Fleet</h1>
  <table>
    <tr>
      <td>Fleet Code</td>
      <td>Download URL</td>
      <td>History URL</td>
      <td>Delete</td>
    </tr>
<? 
$sql = mysql_query("SELECT fleet_id, fleet_code, download_url, history_url FROM 
    imp_fleet");

if (mysql_num_rows($sql) > 0) { 
while ($row = mysql_fetch_array($sql)) {
?>              
    <tr> 
      <td><input type="text" name="fleet_code[]" 
value="<?=$row['fleet_code']?>"><input type="hidden" name="fleet_id[]" 
value="<?=$row['fleet_id']?>"></td>
      <td><input type="text" name="download_url[]" 
value="<?=$row['download_url']?>"></td>
      <td><input type="text" name="history_url[]" 
value="<?=$row['history_url']?>"></td>
      <td><input type="checkbox" name="delete[]" value="<?=$row['fleet_id']?>"></td>   
   
    </tr>
<? 
}
}
?>    
    <tr> 
      <td colsapn="4">
        <table>
          <tr>
            <td><input type="hidden" name="action" value="update"><input type="reset" 
value="cancel"></td>
            <td colspan="2"><input type="submit" value="submit"></td>          
          </tr>
        </table>
      </td>
    </tr>
  </table>
</form>
</body>
</html>

Thanks for your time and feedback.

Matt

Reply via email to