Better code suggestion?

Question

Can someone make my code nicer/easier? I think it's pretty bad coded right now.

I want to call a INSERT MYSQLI command function as following:

DatabaseHandler::__i('TABLENAME', array('username' => 'foobar', 'password' => 'world'));

I am using this now, but it doesn't work, and I think there is a better opportunity.

public function __i($table, $arr) { $c=$this->connection;
   foreach ( $arr as $name => $val):
      $param1.=$name . ",";
      $param2.="?,";
   endforeach;

   $PARAM1 = substr_replace($param1, '', -1);
   $PARAM2 = substr_replace($param2, '', -1);

   $query = 'INSERT INTO '.$table.' ('.$PARAM1.') VALUES ('.$PARAM2.')';

   foreach ( $arr as $name => $val):
    if ( is_int($val)) :
         $param = 'i';
      endif;

      if ( is_string($val)) :
         $param = 's';
      endif;

      if ( is_double($val)) :
         $param = 'd';
      endif;
      $binds.=$param;
      $values.="'".$arr[$name]."',";

    endforeach;

    $values = substr_replace($values, '', -1);

   if ( $s = $c->prepare($query)):

      $args = $binds + $values;
      call_user_func_array(array($s, 'bind_param'), array($binds, $values));
      $s->execute();
      $s->close();
   endif;

}

you need to post this here http://codereview.stackexchange.com/ — amitchhajer, Sep 10 '12 at 11:01
Just an opinion, wouldn't it be easier to make the code "work" first and then try to optimise it? If you change your question to "it doesn't work", then we might be able to get it working first, and then jump with the working code to the link as @amitchhajer suggested — dbf, Sep 10 '12 at 11:04
You should definitively try to pinpoint what exactly is not working first. What errors do you get? — Mahn, Sep 10 '12 at 11:08
Also: http://stackoverflow.com/questions/60174/best-way-to-prevent-sql-injection-in-php — Mahn, Sep 10 '12 at 11:08

score 0 · Answer 1 · answered Sep 10 '12 at 11:20

Hard to tell what's not working, you should find out that yourself, but I would write it something like this:

<?php

/**
 * Prepares and executes an 'INSERT' query
 * 
 * @param string $table
 * @param array $arr
 */
public function __i( $table, $arr ) {

    $c = $this -> connection;

    foreach ( $arr as $name => $val ) {
        $param1 .= $name . ",";
        $param2 .= "?,";
    }

    $PARAM1 = substr_replace( $param1, '', -1 );
    $PARAM2 = substr_replace( $param2, '', -1 );

    $query = 'INSERT INTO ' . $table . ' (' . $PARAM1 . ') VALUES (' . $PARAM2 . ')';

    foreach ( $arr as $name => $val ) {

        switch ( gettype( $val ) ) {
            case 'integer': $param = 'i'; break;
            case 'string': $param = 's'; break;
            case 'double': $param = 'd'; break;
        }

        $binds .= $param;
        $values .= "'" . $arr[$name] . "',";

    }

    $values = substr_replace( $values, '', -1 );

    if ( $s = $c -> prepare( $query ) ) {

        $args = $binds + $values;
        call_user_func_array( array( $s, 'bind_param' ), array( $binds, $values ) );
        $s -> execute();
        $s -> close();
    }

}

?>

Getting this error: Warning: Parameter 2 to mysqli_stmt::bind_param() expected to be a reference, value given in /Applications/XAMPP/xamppfiles/htdocs/classes/databasehandler.php on line 56 while trying calling __insert(array( 'username' => 'testing', 'password' => 'hej123' ), 'users'); — Jesper Kampmann Madsen, Sep 10 '12 at 11:22

Better code suggestion?

1 Answers1