# Class method to insert a record into MySQL

I'm just starting out using OOP; classes and methods etc. I've been looking around for some PDO classes/wrappers out there, but there's not much to choose from. So I've tried to make my own. Started out by writing an "insert" method.

As a man with low self esteem, I never like what I do myself, so I thought I'd ask you guys here for feedback. Here it is:

public function insert($tbl,$data) {

$this->_stmt =$this->_dbh->prepare("INSERT INTO $tbl (" . implode(', ', array_keys($data)) . ") VALUES (:" . implode(', :', array_keys($data)) . ")"); foreach($data as $key =>$value) {
$this->_stmt->bindValue($key, $value, PDO::PARAM_STR); }$this->_stmt->execute();
}


The meaning was that it could all be done in one sweep instead of having several methods to do one thing; inserting a record.

An example of using this code:

$message = 'A message for the ones who like to read it!';$sent_on   = 'Saturday 23rd 2012';
$unique_id = 'unique_as_can_get';$data = array(
'message'   => $message, 'sent_on' =>$sent_on,
'unique_id' => $unique_id );$insert = new DB;
$insert->insert('tablename',$data);


I've put the database connection into the constructor.

The code works as I want it to, but I'm still confused if it's "accepted" amongst you people who are way more skilled that I will ever be.

Please let me know if this code if "usable" and/or what is wrong with it, what can be improved/changed etc.

There's nothing wrong with it, but it's a small piece of code. Some minor details are:

1: I would write $table instead of $tbl, why abbreviate it? You also write foreach ($data as$key => $value), which is very general. Why not specify it better: foreach ($row as $column =>$value)'? What I mean is that variable names should have meaning. I know an array has keys and values, but they are general names. Here you should use names that tell you what a variable really represents.

2: $this->_stmt is a class variable, where a local $statement variable would do. Local variables are always more efficient and have even better encapsulation.

3: Also watch your line length: 156 characters is too much. Instead of writing this:

$this->_stmt =$this->_dbh->prepare("INSERT INTO $tbl (" . implode(', ', array_keys($data)) . ") VALUES (:" .  implode(', :', array_keys($data)) . ")");  (I wrapped it for clarity), you could have written something like; $rowkeys   = array_keys($row);$columns   = implode(',',$rowkeys);$values    = ':'.implode(',:',$rowkeys);$query     = "INSERT INTO $table ($columns) VALUES($values)";$statement = $this->handle->prepare($query);


This makes it easier to read, and debug. It may seem longer, but effectively it does the same thing.

4: You could build in error checks. Is the array a valid array to insert? Start with is_array() for instance. Do the column names exist in the table? Does the insert execute properly? No errors?

5: you could return the lastInsertID()`: http://php.net/manual/en/pdo.lastinsertid.php

6: You cannot insert multiple rows at once with this method. Perhaps you don't need this, but if you do you could add that functionallity.

7: You bind values, so that's quite secure. However, are you sure your column names can never be influenced by outside sources (= hackers)? Another good reason to check them, because they go straight into your SQL command.

8: Please note that wrapping PDO can be a burden later. See: Class for reducing development time If that puts you complete off, don't worry, I also use a wrapper despite all that good advice.

• KIKO Software: Thank you very much! I will take your advices and use them to make it better. Note that this is just the "insert" method for one record; not the entire class. I will take use of transactions in another method if several records should be inserted :) Regards Fredo – FredoSk Mar 16 '15 at 15:04