Coding webpage

Coding Critique is the place to post source code for peer review by other members of DevNetwork. Any kind of code can be posted. Code posted does not have to be limited to PHP. All members are invited to contribute constructive criticism with the goal of improving the code. Posted code should include some background information about it and what areas you specifically would like help with.

Popular code excerpts may be moved to "Code Snippets" by the moderators.

Moderator: General Moderators

Post Reply
devnetworkphp
Forum Newbie
Posts: 3
Joined: Tue Oct 02, 2007 1:35 am

Coding webpage

Post 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.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

Are you asking for a code review?
devnetworkphp
Forum Newbie
Posts: 3
Joined: Tue Oct 02, 2007 1:35 am

Post by devnetworkphp »

Yes, any modifications, better design etc... from experts like you.
User avatar
RobertGonzalez
Site Administrator
Posts: 14293
Joined: Tue Sep 09, 2003 6:04 pm
Location: Fremont, CA, USA

Post by RobertGonzalez »

Moved to Coding Critique.
matthijs
DevNet Master
Posts: 3360
Joined: Thu Oct 06, 2005 3:57 pm

Post 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())
User avatar
superdezign
DevNet Master
Posts: 4135
Joined: Sat Jan 20, 2007 11:06 pm

Post 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.
Post Reply