0

I am adding some server-side form validations (using php) in case one of the users of my site has javascript turned off. On one form, there are 10 separate input fields that can be changed. Could someone please tell me which protocol will use less system resources? In the first, I write some mySQL variables to check the user's current settings, and compare these with the posted settings. If all 10 posted values are identical to the current values, don't UPDATE database, else UPDATE the database:

$login_id = $_SESSION['login_id'];

$sql1 = mysql_fetch_assoc(mysql_query("SELECT value1 FROM login WHERE login_id = 
'$login_id'"));
$sql1a = $sql1['value1'];
// Eight More, then
$sql10 = mysql_fetch_assoc(mysql_query("SELECT value10 FROM login WHERE login_id = 
'$login_id'"));
$sql10a = $sql10['value10'];
$Value1 = $_POST['Value1'];
// Eight More, then
$Value10 = $_POST['Value10'];

//Other validations then the following

if (($sql1a == $Value1)&&($sql2a == $Value2)&&.......($sql10a == $Value10)) {
echo "<script>
alert ('You haven't made any changes to your profile');
location = 'currentpage.php';
</script>";
}
else {
$sqlUpdate = mysql_query("UPDATE login SET value1 = '$Value1',....value10 = '$Value10'     
WHERE login_id = '$login_id'");
echo "<script>
alert ('Your profile has been updated!');
location = 'currentpage.php';
</script>"; 
}//End php

OR is it less expensive to just use the user-posted values (keep the $_POST variables) and avoid checking with the comparison line: (($sql1a == $Value1)&&($sql2a == $Value2)&&.......($sql10a == $Value10)) and just go right to

//Other validations then the following

$sqlUpdate = mysql_query("UPDATE login SET value1 = '$Value1',....value10 = '$Value10'     
WHERE login_id = '$login_id'");
echo "<script>
alert ('Your profile has been updated!');
location = 'currentpage.php';
</script>"; 

Thanks for any input on this!

  • 2
    There's bound to be somehting intelligent to be said about it theoretically, but why not test it? the only real way to find out something about performance is testing! – Nanne Nov 14 '13 at 19:48
  • Save the user object or array to the session instead of just the login_id, then compare the form values to the session values and if they change mark it as dirty. Then if the session user is dirty update. I would not pull from the database 10 times for each form submission. – Tony Nov 14 '13 at 19:56
  • OK @Nanne...I'll ask the question which betrays my rookie status with servers....how would I do this? I am able to check bandwidth with my webhosting service (finest resolution I can get is 30-minute blocks over a 24 hour period) and I can check mySQL Usage (just shows as far as I can tell how much space my stored data occupies). – The One and Only ChemistryBlob Nov 14 '13 at 19:59
  • OK...thanks @Tony. I will contemplate this and apply changes where I can. Thanks for your help! – The One and Only ChemistryBlob Nov 14 '13 at 20:04
  • 1
    The first O'Brien you get as a client is going to ruin your day. Use parameterized queries instead, as seen in the example here: http://www.php.net/manual/en/pdo.prepared-statements.php . Also, mysql_(whatever) is deprecated; don't use it. Try PDO like in that link; it'll make your life way easier. – aehiilrs Nov 14 '13 at 20:48
  • Ahm.. why exactly do you need to check if the form has different values? If the form is meant for changing values then simply insert the new values (after data validation ofc). If user simply submits the form with current settings.. you simply do a normal update and old values will be written again. What is the problem? Also, you are writing 10 queries to retreive 10 fields from the same table!!! You only need one query! – cen Nov 14 '13 at 21:48

1 Answers1

2

If I understand correctly, your question is whether it's OK for performance to check the profile for modifications. For me, after I've checked your code, this is about much more than just performance...

  • Let's start with the performance: AFAIK MySQL queries are slower than basic PHP comparisions, that's true - but on this scale, I really don't think it matters much. We're talking about two very basic queries which won't handle a lot of data.
  • Let's think about what the end user will see (UX): in the second scenario, the user will not have the most exact feedback telling him/her that no modification has been done. On a profile modification screen, I suppose that might not be intentional, so I would tell that we haven't modified anything. (Also, performing an unnecessary UPDATE query is not the most elegant.)
  • @aehiilrs is right, please pay attention to that comment. This style of MySQL usage is particularly bad for security - if you keep going with this, you will create a lot of security holes in your PHP code. And those are really easy to discover and exploit, so please, have a good look on the alternatives, starting with PDO as mentioned. Any good PHP book out there will show you the way. You can also have a look at a great Q/A here on StackOverflow: How can I prevent SQL injection in PHP?
  • I wonder whether it's a good idea to try to update the user interface like you did - I would strongly prefer loading another PHP without any <script> magic in the output. In the result PHP, you can always display something like a CSS-styled statusbar for displaying info like that.
Community
  • 1
  • 1
Scorchio
  • 2,763
  • 2
  • 20
  • 28
  • Thank you for the great answer. I have included (not shown in code above...grouped under "Other validations then the following") lots of checks in my php code, including multiple instances of preg_match, substr, strstr, mysql_real_escape_string, and stripslashes. No special characters allowed for any username...e.g., O'Brien could not be a username! – The One and Only ChemistryBlob Nov 14 '13 at 22:29
  • @ChemBlob9999 That's cool. What have you forgotten to check for? – aehiilrs Nov 14 '13 at 23:25
  • I'm sure a lot of things...I'm still learning, which is why I ask a lot of questions on this site :) I know you are correct...I have to change my mysql statements to mysqli – The One and Only ChemistryBlob Nov 14 '13 at 23:35
  • 1
    Excellent. Concatenating strings entered by the user into an SQL statement is asking for trouble - I've done it before and even though it seems quicker, it always leads to pain. Make sure you use bound parameters rather than "select * from table where column = '$whatever';", though. Manually sanitizing each field for SQL injection problems is a lot of work, especially when there's a library that lets you avoid the problem entirely. – aehiilrs Nov 14 '13 at 23:41
  • @ChemBlob9999 I've added a link pointing to a question here on StackOverflow, which will explain a lot for you on how to make a transition to more secure queries. – Scorchio Nov 15 '13 at 06:03