current community

your communities

more stack exchange communities

Stack Exchange
tour help
careers 2.0
Take the 2-minute tour ×
Stack Overflow is a question and answer site for professional and enthusiast programmers. It's 100% free, no registration required.
up vote 1 down vote favorite 
$dblocation = "localhost";
$dbname = "xx";
$dbuser = "xx";
$dbpasswd = "xx";
$dbcnx = @mysql_connect($dblocation,$dbuser,$dbpasswd);
mysql_select_db($dbname, $dbcnx);
mysql_query("SET NAMES utf8");
mysql_query("SET COLLATION_CONNECTION=utf8_bin"); 
$num = mysql_query("select no from votes WHERE id='".$id."';");
$num2 = $num + 1;
$num3 = mysql_real_escape_string($num2);
mysql_query("UPDATE votes SET no='".$num3."' WHERE id=".$id.";");

It's a php code. I want it to:
1) Get a "no" value (INT) from "votes" table from the row where "id" (INT) equals $id
2) Increment this value by one
3) Write the new value to "votes" table to the row with "id" (INT) equals $id

But somehow my code is not working properly. Any ideas?

share|improve this question
 
Is your PHP returning any errors? –  mc10 Jun 3 '11 at 17:27
 
Nope, everything seems fine. Except it's not doing anything with database. User is granted to do whatever it needs with the "votes" table, also selecting from db works perfectly fine. Something wrong with "Update" query, I guess. –  Adiost Jun 3 '11 at 17:29
 
Run queries in the db without PHP to see if queries return the result. And why don't you just do an update statement "UPDATE votes SET no=(no+1) WHERE id='$id'"; - it doesn't make any sense to retrieve a numeric value, increase it in php and then update a db - it's all done quickly in one query. –  AR. Jun 3 '11 at 17:32
add comment

4 Answers

up vote 1 down vote accepted

You should, for starters, get rid of the @ symbols in front of function calls. This will suppress errors caused by the given statement - and leave you wondering what is going wrong. While you are developing, you want all the information about your code you can get, especially errors. So, removing the @ (error suppression operator) is the first step.

Second, whenever you query your database, you have two steps. First you call mysql_query. Then, after ensuring that mysql_query returned anything, you call a fetch-type statement on the query object. Check out the docs for mysql_query: http://php.net/manual/en/function.mysql-query.php There is a great, simple example there - see if you can spot where your code goes wrong.

All of that aside, don't need to jump through all those hoops to increment a database field...

mysql_query('UPDATE votes SET no = no+1 WHERE id='.$id);

For reference, a quick re-write of your sample code would look like this (using your approach rather than the one I mention above):

$dblocation = "localhost";
$dbname = "xx";
$dbuser = "xx";
$dbpasswd = "xx";
$dbcnx = mysql_connect($dblocation,$dbuser,$dbpasswd);
// handle a problem connecting to the database.  You probably want to do something nicer than just "die" an error message
if (!$dbcnx)
    die('Error connecting to database');

mysql_select_db($dbname, $dbcnx);

// not strictly neccesary to do this
mysql_query("SET NAMES utf8");
mysql_query("SET COLLATION_CONNECTION=utf8_bin");


$query = mysql_query('SELECT `no` FROM `votes` WHERE `id`='.$id);
// again, you'll probably want to do something nicer than blat out a text error
if (!$query)
    die('Record not found');

$num = mysql_fetch_assoc($query);
// type your number
$num = (int) $num['no'];

// increment it
$num++;

mysql_query('UPDATE `votes` SET `no`='.$num.' WHERE `id`='.$id);
share|improve this answer
 
Man, you're awesome. Thanks. –  Adiost Jun 3 '11 at 17:43
add comment
up vote 2 down vote

There are several things that can go wrong here -

  • You are suppressing errors from mysql_connect() - it might not be connecting properly to the database
  • mysql_query() returns a resource which then has to be used by one of the row fetching functions (such as mysql_fetch_assoc() ). You can't use its result like you do.

Basically, you should be running your code with the highest error_reporting level during development. You will know immediately what the problem is (and drop the error suppression operator in front of mysql_connect() - there is almost never a good reason to use it).

As an aside, there is no reason to set collation and character set for retrieving numbers. This entire operation can be replaced by a single update query:

$query = "UPDATE votes SET no=no+1 WHERE id=" . (int) $id;
share|improve this answer
add comment
up vote 0 down vote

You missed to get the row from the result:

$result= mysql_query("select no from votes WHERE id='".$id."';");
// you have to fetch a row from the result
$row = mysql_fetch_array($result);
// your first item in the array will be the value of 'no'
$num2 = $row[0] + 1;
share|improve this answer
 
Also assumes that result is returned... need to check to make sure you have a row to fetch before you call mysql_fetch_array! –  Chris Jun 3 '11 at 17:34
 
@Chris you're right. –  aorcsik Jun 3 '11 at 17:38
add comment
up vote 0 down vote

There are any number of things that can co wrong without you knowing, put a few ifs and die()s (check if $dbcnx is not null).

Next, $num is actually a resource, not a result. To get the result, do the following:

$result = mysql_query("select no from votes WHERE id='".$id."';");
$row = mysql_fetch_assoc($result);
$num = (int) $row['no'];

Lastly, the "no" column looks like it is an integer, so you do not need the single quotes around it in the query statement, nor do you need to escape it like you do with text, just cast it as an int and that will eliminate all risk.

share|improve this answer
 
This code simply assumes that a result has been returned... what if no row is found with the given ID? –  Chris Jun 3 '11 at 17:33
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.