jjinno — Wed, 03 Aug 2005 01:07:31 +0000

aboyd wrote: You took $siteid directly from $_POST!

Dont be so shocked. Coding in general should be done iteratively and incrementally, and each test-case accounted for.

If I know he is passing a number, should I check to see if a lazy "?" floated in the mix? Probably completely unnecessary...probably.

But if you are insistant...Since he never passes anything but 0-9, then a simpler "else die" would suffice.

aboyd — Sun, 24 Jul 2005 08:31:52 +0000

Python wrote: $result = mysql_query("SELECT `rating`, `numvotes` FROM `reviews_site` WHERE `id` = $siteid");

:::boggle:::

You took $siteid directly from $_POST! PHP is pretty good at preventing SQL injection, but shouldn't that data get SOME laundering? Even just this:

$siteid = preg_replace('/[^0-9]+/', '', $siteid);

-Tony

Greg K — Sun, 24 Jul 2005 01:42:49 +0000

First, see http://us2.php.net/manual/en/language.types.array.php#language.types.array.donts for why to use $_POST['siteid'] instead of $_POST[siteid]

As to why your database isn't updating. Here is my #1 suggestion to anyone who says a database isn't updating right (or not returning data you expect).

Right before the line with mysql_query add this line:

<?php
print \"<tt><pre>\nSQL=$query\n</pre></tt>\n\";
?>

This will display the query string as it is being sent to the server. The

 is to help determine any spacing problems, not really needed but I like them. Once you get the bugs out, comment out this line.
   
 If this looks right, my suggestion then would be to go into phpMyAdmin (or my preference, log into the DB server itself) and manually execute the sql statement displayed by the above. This sometimes will give you better errors, and see results better if you don't have them coded properly to give results.
   
   However, in this case, I took your code, copied and pasted it into Zend Studio, told it to analize it, and bingo...  
   
   Global variable $final  was used before it was defined (line 16)
   ( I added a <? line, so line 16 was the line where you define the $query)
   
 You were probably wanting $final2 in the SQL statement. If you added the print statement above, you would have seen that the value to update the database with was missing.
   
   Also, in your update statement, you need to separate the values with a comma, not &&:
   
   <?php
$query = \"UPDATE `reviews_site` SET `rating` = $final, `numvotes` = $newnumvotes WHERE `id` = $siteid\";
?>
  BTW, to make coding more simplified, you can reduce your lines to get the data from the database (the whole while statement) to just this line:
   <?php
list($currating,$curnumvotes) = mysql_fetch_assoc($result);
?>
   I'm assuming that there should only be one record with each site ID.
   
 Next, lets take a look at your math logic behind your ratings. You are taking the previous rating, adding the current rating, then dividing by the total # of ratings. This will (after the first 2 ratings) give you the wrong numbers. You need to take the total of ALL the ratings then divide them to get the average. So in your database, you should be storing TotalRating and NumVotes, then when you display the rating, do 
   <?php
$query = \"SELECT (`TotalRating`/`NumVotes`) AS AvgRating FROM `reviews_site` WHERE `id` = $siteid\";
   $result = mysql_query($query);
   list($avgRating) = mysql_fetch_array($result);
?>
   
   See the image below (or look here if this doesn't display right) to see sample data and how it would calculate differently.
   
   
   As you can see, the more you are rated, the worst your rating will be. It can never be higher than 10 (possible 5 for previous average, plus current 5) divided by total votes. And actually, after the 3rd vote, it can never reach 10. Even if you have a perfect vote of 5 each time, here is what you end up with: (alt link if img doesn't display here)
  

 Lastly, I would recommend breaking up your beginning sql statements into a $query variable and then a line to execute the sql. Then if needed you could the print statement if needed.
   
   Good luck with your code, let us know if you need any more help.
   
   -Greg