* Thus wrote Matthew Oatham ([EMAIL PROTECTED]):
> 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?
>
>
> <?
use full php tags: <?php
> if (isset($_POST['delete'])) {
> $deleteList = join(', ', $_POST['delete']);
be careful here! if I add to the _POST data something like
&delete%5B%5D=%29%20OR%281
then I just deleted all your records.
> //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];
I would set up your array's so they are a little more readable and
dont rely on autoincrementing array key's. So your loop would look
something like this:
foreach($_POST['fleets'] as $fleet_id => $fleet) {
echo $fleet_id; // the fleet_id
echo $fleet['code'];
echo $fleet['urls']['history'];
echo $fleet['ulrs']['download'];
...
> mysql_query("UPDATE imp_fleet SET fleet_code = '$fleetCode', history_url =
> '$historyUrl', download_url = '$downloadUrl' WHERE fleet_id = $value") or die
> (mysql_error());
escape your data that is untrusted, see http://php.net/mysql_real_escape_string
> }
> if ($deleteList) {
> mysql_query("DELETE FROM imp_fleet WHERE fleet_id IN($deleteList)") or die
> (mysql_error());
> }
> if (mysql_error()) {
this is never reached if there was an error since you die'd() on
errors befor.
> }
> }
> ...
>
> 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>
>
// set up a variable for the input form names for easier reading.
$var_name = 'fleets['.$row['fleet_id'].']';
?>
<td>
<input name="<?php echo $var_name?>[code]">
<input name="<?php echo $var_name?>[urls][history]">
<input name="<?php echo $var_name?>[urls][download]">
...
I'd suggest not using the <?= shorthand also.
HTH,
Curt
--
"I used to think I was indecisive, but now I'm not so sure."
--
PHP General Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php
