Looking for a simpler way

[dmcglone]

I don't think I fully understand MySQL or PHP for that matter. LOL  but anyhow, I have written a function that will check to see if a value is in the db and if not, insert the record and if not, then don't and I believe there is probably a much easier way to do this without having to pull out all these steps I have. Can anyone educate me here a little bit and show me how they would accomplish this task in a much shorter or simpler way?

Here's the code I have written:

function insert(){
      $this->vin = $_POST['vin'];
      $result = mysqli_query($this->myconn, "SELECT * FROM inventory");
      while ($row = mysqli_fetch_array($result)){
      $vins = $row[7];
      }
 
      if($this->vin != $vins ){
 
$insert = "INSERT INTO inventory (vin) VALUES ('$_POST[vin]')";
if(!mysqli_query($this->myconn, $insert)){
die('Error: ' . mysqli_error($this->myconn));
      }
echo "added vin # " . $this->vin;
 
      } else {
 
      echo $this->vin . " is already in the database";
     }
   }

 

Blessings

David M.

[AbraCadaver]

No reason for a loop of all records.  The database will return what you want and then you can check it.  Just query to see if the one record is there:

SELECT * FROM inventory WHERE vin = $_POST['vin']

And then check mysqli_num_rows() to see if you got results.  If not, inset and if so don't.  You should still escape the $_POST data with mysqli_real_escape_string().

[dmcglone]

Hmmm, I spent all day Tuesday trying it the way you suggest and for some reason I wasn't getting the correct results. I finally changed it around last night to something I understood better and I wasn't happy with myself knowing I should have been able to do it in less steps.

I think one reason for my confusion is probably because this is the first time I've ever sat down and tried to use mysqli and I'm not comfortable with it yet.

 

Anyway later today I'm going to write it again like you suggest and maybe this time around I'll spot where I was going wrong the first time.

Blessings

David M.

[mikosiko]

if you are trying to not insert records with a duplicated "VIN", means then that you can define a UNIQUE index in that field (if it is not a PK already), and then you can simplify your logic using only an INSERT...IGNORE sentence and controlling/checking the outcome with mysqli_affected_rows()... something like this:  (Abracadaver already told you to escape your $_POST[] before to use it directly in the query)

$insert = "INSERT IGNORE INTO inventory (vin) VALUES ('$_POST[vin]')";
$result = mysqli_query($this->myconn, $insert);

if ( !mysqli_affected_rows($this->myconn) ) {
        // Nothing was inserted... mysqli_affected_rows() return 0... means most likely a duplicated vin was found
      ... do whatever you want here
} else {
   // the Insert was successful
    ... proceed..
}

you can read here about INSERT...IGNORE format

[Psycho]

Also, you should pass the value to the function as a parameter instead of having the function directly reference the POST value. That way you can re purpose the function as needed. I would also suggest giving the function a better name, such as insertVin() so it is obvious what it is for. Plus, it is considered poor form to echo within a function.

 

Here is an example script. Note that INSERT IGNORE will not produce an error for any problems - not just duplicate keys. For a simple query such as this, it should be fine. But, for more complicated scenarios using it could make debugging very difficult.

function insertVin($vin)
{
    //Trim and escape the value
    $vinValue = mysql_real_escape_string(trim($vin));
    //Verify value exists
    if(empty($vinValue)) { return false; }
    //Attempt to insert the record
    $query = "INSERT IGNORE INTO inventory (vin) VALUES ('$vinValue')";
    $result = mysqli_query($this->myconn, $query);
    //Check if the record was inserted
    if(mysqli_affected_rows($this->myconn)>0)
    {
        return "added vin # {$vin}";
    }
    else
    {
        return "{$vin} is already in the database";
    }
}

//Usage
$isertVinResult = insertVin($_POST['vin']);
if(!$isertVinResult)
{
    echo "There was a problem attempting to insert the vin '{$_POST['vin']}'";
}
else
{
    echo $isertVinResult;
}

Edited August 29, 2013 by Psycho

[dmcglone]

Thanks guys for the responses. I'm about to play around with the examples and try to understand them.

Psycho, although this is completely for my own learning, aside from giving an example, thank  you for the explanations. I think I got more excited over them than the code itself.   

Edited August 30, 2013 by dmcglone

[dmcglone]

Hello guys.

I finally got some time to play around with psycho's example. I have only slightly modified it to see if what I could do. The only problem is, if the vin number does exist in the db, it's still adding that number instead of executing the else condition and I'm wondering what could be causing it.
 

 
//sql_functions.php
 
class Connection {
 
  //Create the connection to mysql
  public function __construct() {
    $this->conn= mysqli_connect(DB_HOST,DB_USER,DB_PASS);
    if(!$this->conn){
      die ("Cannot connect to the database " .DB_NAME);
    }
 
    //after making a connection to mysql, we then select which database we want to use.
    mysqli_select_db($this->conn, DB_NAME);
    if(mysql_error()){
      echo "Cannot find the database table ";
      exit();
    }else {
      echo "Database " .DB_NAME. " selected <p />";
  }
 }
 
 function insertVin($vin) {
   //Trim and escape the value
   $vinValue = mysql_real_escape_string(trim($vin));
   //Verify value exists
   if(empty($vinValue)) { return false; }
   //Attempt to insert the record
   $query = "INSERT IGNORE INTO inventory (vin) VALUES ('$vinValue')";
   $result = mysqli_query($this->conn, $query);
   //Check if the record was inserted
   if(mysqli_affected_rows($this->conn)>0)
   {
     return "added vin # {$vin}";
     }
     else
     {
       return "{$vin} is already in the database";
       }
     }
 
}

 

//test_sql_functions.php
 

require_once "errorReporting.inc.php";
require_once "sql_functions.php";
 
$connection = new Connection();
 
$isertVinResult = $connection->insertVin($_POST['vin']);
if(!$isertVinResult)
{
  echo "There was a problem attempting to insert the vin '{$_POST['vin']}'";
  }
  else
  {
    echo "$isertVinResult correctly";
  }

 

[dmcglone]

last post was a little pre-mature. I found my answer on how to use mysql_affect_rows. Here's what I changed the function to, but the problem of adding duplicates is still  there, but now I think I see why.
 

 

function insertVin($vin) {
   //Trim and escape the value
   $vinValue = mysql_real_escape_string(trim($vin));
   //Verify value exists
   if(empty($vinValue)) { return false; }
   //Attempt to insert the record
   $query = "INSERT IGNORE INTO inventory (vin) VALUES ('$vinValue')";
   $result = mysqli_query($this->conn, $query);
   $affected_rows = mysqli_affected_rows($this->conn);
   echo "total # of rows affect by this insert " . $affected_rows . "<p>";
   //Check if the record was inserted
   if($affected_rows>0)
   {
     return "added vin # {$vin}";
     }
     else
     {
       return "{$vin} is already in the database";
       }
     }
 
}

[dmcglone]

Haha Okie doke. I think I see what's happening now. I  think the function is evaluating only if a row is inserted which will always be true, because I don't have any checks to compare whether the same vin already exists in the DB. Once I write some code to check whether the vin already exists, then I will then be able to stop the insertion if it already exist. :-)

Thoughts anyone?

[kicken]

but the problem of adding duplicates is still  there, but now I think I see why.

That code relies on your vin column being defined as a UNIQUE index. Without that it will happily keep inserting duplicate rows.

[dmcglone]

That code relies on your vin column being defined as a UNIQUE index. Without that it will happily keep inserting duplicate rows.

Well I'll be darned! I thought I had marked that column as unique the other day, but it refused to make the changes because there were duplicates and I didn't realize it.. It's working correctly now... 

 

Thank you for the heads up. took me a while to figure this one out, but I learned a whole lot. :-)