I ran across this piece of code in a program I am updating. The original author is a great game designer, but only a fair programmer. He also seems to have very limited experience with SQL.

PLAIN TEXT
PHP:
  1. if ( $grab == 'Ammo' ) {
  2.     $check = mysql_query("SELECT * FROM BF WHERE owner= 'None' AND country='$fetch->location'");
  3.     $num_rows = mysql_num_rows($check);
  4.  
  5.     if ( $num_rows != 0 ) {
  6.         if ( $fetch->location == $location ) {
  7.             mysql_query("UPDATE BF SET owner='$username', profit='0' WHERE country='$fetch->location' LIMIT 1");
  8.             $this->content .= "You got the Bullet Factory!";
  9.         }
  10.     }
  11. }

When writing database applications, you need to be as good at writing queries as you are at writing code. Here's a rewrite:

PLAIN TEXT
PHP:
  1. if ( $grab == 'Ammo' ) {
  2.  
  3.     if ( $fetch->location == $location ) {
  4.  
  5.         $query = sprintf(
  6.             "UPDATE BF SET owner='%s', profit='0' WHERE owner='None' AND country='%s'",
  7.             mysql_real_escape_string(username),
  8.             mysql_real_escape_string($location)
  9.         );
  10.         $result = mysql_query($query);
  11.  
  12.         if ( mysql_affected_rows($result) ) {
  13.             $this->content .= "You got the Bullet Factory!";
  14.         }
  15.     }
  16. }

Now I use just one query. If the update is unsuccessful then no rows will be affected and I won't update the content. Whenever you do a query, check some data and then do an update, look for ways to have the database do the variable checking.

In this case, the author was comparing $fetch->location and $location, but first he was checking for a record where the country field matched $fetch->location. I took the innermost test and made it the outermost test. Now, $fetch->location, the country where the user is located and $location, the location of the Bullet Factory are compared first.

That test does not involve a database hit. I only hit the database if the locations match. Then I only hit it once. If no rows were updated, mysql_affected_rows() returns 0 and content is not appended.

Eventually, I will be writing an object for accessing the BF (Bullet Factory) table. Then this will be a method of that object. It might look something like this.

PLAIN TEXT
PHP:
  1. public function buyFactory($user_location, $username) {
  2.  
  3.     if ( $this->location != $user_location ) {
  4.         return 0;
  5.     }
  6.  
  7.     $query = sprintf(
  8.         "UPDATE BF SET owner='%s', profit='0' WHERE owner='None' AND country='%s'",
  9.         mysql_real_escape_string(username),
  10.         mysql_real_escape_string($this->location)
  11.     );
  12.     $result = mysql_query($query);
  13.  
  14.     return mysql_affected_rows($result);
  15. }

Digg!