Amit20 Posted September 27, 2011 Share Posted September 27, 2011 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???? Quote Link to comment https://forums.phpfreaks.com/topic/247966-just-need-your-reviews/ Share on other sites More sharing options...
gizmola Posted September 27, 2011 Share Posted September 27, 2011 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(). Quote Link to comment https://forums.phpfreaks.com/topic/247966-just-need-your-reviews/#findComment-1273301 Share on other sites More sharing options...
Amit20 Posted September 27, 2011 Author Share Posted September 27, 2011 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??? Quote Link to comment https://forums.phpfreaks.com/topic/247966-just-need-your-reviews/#findComment-1273306 Share on other sites More sharing options...
Psycho Posted September 27, 2011 Share Posted September 27, 2011 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()); Quote Link to comment https://forums.phpfreaks.com/topic/247966-just-need-your-reviews/#findComment-1273313 Share on other sites More sharing options...
Amit20 Posted September 27, 2011 Author Share Posted September 27, 2011 Thank You very much Ok i will build a query through loop. Thank you once again for your help Quote Link to comment https://forums.phpfreaks.com/topic/247966-just-need-your-reviews/#findComment-1273316 Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.