0

Hi I am inserting image data into a database each time an image is uploaded to my server. The code I am using looks a bit 'chunky' especially the binds. Can it be done differently to reduce the amount of text and execute more quickly or should I not worry about it?

Here is the code I am using:

function($file_name, $cat, $year, $desc, $title, $image_size, $image_width, $image_height){
    //test the connection
    try {
        //connect to the database
        $dbh = new PDO("mysql:host=localhost;dbname=mjbox","root", "usbw");
        //if there is an error catch it here
    } catch( PDOException $e ) {
        //display the error
        echo $e->getMessage();
    }

    $stmt = $dbh->prepare("INSERT INTO mjbox_images(img_file_name,img_cat,
        img_year,img_desc,img_title,img_size,img_width,img_height) 
        VALUES(?,?,?,?,?,?,?,?)");

        $stmt->bindParam(1,$file_name);
        $stmt->bindParam(2,$cat);
        $stmt->bindParam(3,$year);
        $stmt->bindParam(4,$desc);
        $stmt->bindParam(5,$title);
        $stmt->bindParam(6,$image_size);
        $stmt->bindParam(7,$image_width);
        $stmt->bindParam(8,$image_height);
        $stmt->execute();
    }
Ja͢ck
  • 170,779
  • 38
  • 263
  • 309
crmepham
  • 4,676
  • 19
  • 80
  • 155
  • 1
    You could simply add all values to an array and execute that. Although this has nothing to do with efficiency as per your title. – PeeHaa Jun 04 '12 at 14:49
  • You may also want to check out how to [disable the emulating of prepared statements](http://stackoverflow.com/a/60496/508666). – PeeHaa Jun 04 '12 at 14:51
  • you should probably return in your catch block as you'd be running a query on a junk object instance of PDO (which failed to connect, as the PDOException would be telling you). – damianb Jun 04 '12 at 14:52

2 Answers2

-1

Depending on how much code you want to rewrite, you could always swap from using pure PDO to something like RedBean (which is actually quite nice, being a zero-config ORM).

http://www.redbeanphp.com/

Worth a look, even if you won't use it now; it's definitely a great tool.

Inserts would then take just modifying bean properties, reducing the overall amount of code you'd use.

damianb
  • 1,224
  • 7
  • 16
-2

You could do it like this, passing an array of values and use the keys as place holders, that way you can use the same function to insert into different tables:

<?php 
$insert = array('img_file_name'=>'',
                'img_cat'=>'',
                'img_year'=>'',
                'img_desc'=>'',
                'img_title'=>'',
                'img_size'=>'',
                'img_width'=>'',
                'img_height'=>'');

insert('mjbox_images',$insert);

function insert($table,$values=array()){

    //test the connection
    try{
        //connect to the database
        $dbh = new PDO("mysql:host=localhost;dbname=mjbox","root", "usbw");
        //if there is an error catch it here
    } catch( PDOException $e ) {
        //display the error
        echo $e->getMessage();
    }

    $fieldnames = array_keys($values);

    $sql = "INSERT INTO $table";
    $fields = '( ' . implode(' ,', $fieldnames) . ' )';
    $bound = '(:' . implode(', :', $fieldnames) . ' )';
    $sql .= $fields.' VALUES '.$bound;

    $stmt = $dbh->prepare($sql);
    $stmt->execute($values);// whoops
}

//INSERT INTO mjbox_images( img_file_name ,img_cat ,img_year ,img_desc ,img_title ,img_size ,img_width ,img_height ) VALUES (:img_file_name, :img_cat, :img_year, :img_desc, :img_title, :img_size, :img_width, :img_height )

?>
Lawrence Cherone
  • 46,049
  • 7
  • 62
  • 106
  • Are you going to open a new connection every time the user wants to insert something? And what about updating, deleting etc? – PeeHaa Jun 04 '12 at 15:04
  • 1
    Is writing a complete CRUD for the OP expected of me? – Lawrence Cherone Jun 04 '12 at 15:07
  • For a question with efficient in the title I expect the answer to be efficient. Which this isn't. Also nothing is inserted in that `$stmt->execute()` call. And I'm sorry singletons are not really any better. – PeeHaa Jun 04 '12 at 15:14