Page 1 of 1

Coding webpage

Posted: Sun Oct 14, 2007 9:02 am
by devnetworkphp

Code: Select all

<?php
require_once('db.php');

$act=$_GET['act'];
$id=$_GET['id'];


if($act=='insert')
{

  $username=$_POST['username'];
  if(empty($username))
    $error='Please enter username';
   
  if(!empty($error))
 {
    $act='form';
  }
 else
{
  if(empty($id) && empty($error))
    $query=mysql_query("UPDATE userlist SET username='$username' WHERE id=$id");
  else
    $query=mysql_query("Insert INTO userlist SET username='$username'");

       if(!empty($query) && !empty($id))           $message='Updated';
       elseif(!empty($query) && empty($id))      $message='Inserted';
 }
}

if($act=='delete')
{
   $query=mysql_query("DELETE FROM username WHERE id=$id");
   if($query)  $message='Deleted';
        
}

if($act=='form')
{
   if(!empty($id))
    {
       $query=mysql_query("SELECT username FROM userdetails WHERE id=$id");
       $fetch_details=mysql_fetch_array($query);
       $username=$fetch_details['username'];
    }
  
   $form="<form method='post' action='userdet.php?act=insert&id=$id'>
                     <input type='text' name='username' value='$username'>
                      <input type='submit' name='submit' value='submit'>
   </form>";

}

if($act=='page')
{
  // paging 

}


echo "<table><tr><td>$message $error</td></tr><tr><td>$form</td></tr></table>";

?>
Please comment if there is any modifications that should be done in the above design.

Posted: Sun Oct 14, 2007 9:42 am
by RobertGonzalez
Are you asking for a code review?

Posted: Sun Oct 14, 2007 10:15 am
by devnetworkphp
Yes, any modifications, better design etc... from experts like you.

Posted: Sun Oct 14, 2007 10:38 am
by RobertGonzalez
Moved to Coding Critique.

Posted: Sun Oct 14, 2007 1:06 pm
by matthijs
I would 1) validate the incoming data 2) escape the data going into the database (mysql_real_escape_string()) and 3) escape the output to the browser (htmlentities())

Posted: Sun Oct 14, 2007 2:19 pm
by superdezign
I'd just organize the if statements as a switch, and add in a default action.

I'd write the whole thing differently, but readability is a big issue with me, and I've noticed that different programmers read code differently.