Just need your reviews

[Amit20]

Hello Friends i just need your reviews for the following code

 

<?php 
   session_start();
   require_once("conf.php");
   mysql_select_db('site');
   $subjects=$_GET['sublen'];
   $username=$_SESSION['user'];
  for($i=1;$i<=$subjects;$i++)
  {
     $subject=$_GET[',Sub'.$i];
     $query="INSERT INTO Subjects(id,Subjects,User,Classs) VALUES($i,'$subject','$username','F.Y.B.Sc')";
     $true=mysql_query($query) or die("Cannot Store subjects".mysql_error());
   }

?>

 

Is this a valid way to store data in database????

[gizmola]

First off, why are you making your own id value?  Does this table not have an auto_increment key?

 

Second, it is more efficient to do multiple inserts in one transaction:

 

INSERT into Subjects (Subjects, User, Classs) VALUES ('$subject1', '$username', '...'), ('$subject2', '$username', '...'), etc...

 

So it would be better if your loop added a set of VALUES to the $query string each time through the loop, and when finished, you need only do one mysql_query().

[Amit20]

Thank you for your reply

 

Firstly, i do have an Auto-increment value, i wont be inserting it manually.

 

Secondly, I like your idea of using multiple inserts

INSERT into Subjects (Subjects, User, Classs) VALUES ('$subject1', '$username', '...'), ('$subject2', '$username', '...'), etc...

 

but what if i have say around 50 entries at a time..

 

Is my previous code reliable???

[Psycho]

Is my previous code reliable???

 

Reliable? Well, it won't be efficient which could lead to some problems. Do ONE query to insert all your records. You can use the loop to "build" the query.

 

Do not pass a value to determine how many subjects are being passed. Instead, create the subject input fields as an array

Subject 1: <input type="text" name="subjects[]">
Subject 2: <input type="text" name="subjects[]">
Subject 3: <input type="text" name="subjects[]">
...

 

Then in your processing logic, loop over the array and determine if a value was passed. If so, add it to your insert data, otherwise skip it. I would also advise sending the data as POST values, but you can use GET if absolutely necessary. But, you will want to urlencode()/urldecode() if you do that.

 

 

Sample code

session_start();
require_once("conf.php");
mysql_select_db('site');

$username = mysql_real_escape_string(trim($_SESSION['user']));

$values = array();
foreach($_POST['subjects'] as $subject)
{
    $subject = mysql_real_escape_string(trim($subject));
    if(!empty($subject))
    {
        $values[] = "('{$subject}', '{$username}', 'F.Y.B.Sc')";
    }
}
$query = "INSERT INTO Subjects (`Subjects`, `User`, `Classs`) VALUES " . implode(', ', $values);
$result = mysql_query($query) or die("Cannot Store subjects ".mysql_error());

[Amit20]

Thank You very much

 

 

Ok i will build a query through loop.

 

Thank you once again for your help