current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 3 down vote favorite
1

I wrote a class with this function. Can I improve my code performance in the bottom of the function? I don't like the while loops!

public function loadFields(array $fields, $as_object = Config::FIELDS_AS_OBJECT) {
        $connection = $this->connection;
        $sql = "SELECT ".implode(",", $fields)." FROM ".$this->table." WHERE lang = ?";
        if (!($stmt = $connection->prepare($sql))){
            throw new MysqliPrepareException($connection);
        }
        if(!($stmt->bind_param('s',$this->lang))){
            throw new MysqliBindParamException($connection);
        }
        if(!$stmt->execute()){
            throw new MysqliExecuteException($connection);
        }
        $stmt->store_result();
        $variables = array();
        $data = array();
        $meta = $stmt->result_metadata();
        while ($field = $meta->fetch_field()) {
            $variables[] = &$data[$field->name];
        }
        call_user_func_array(array($stmt, 'bind_result'), $variables);
        $i = 0;
        while ($stmt->fetch()) {
            $array[$i] = array();
            foreach ($data as $k => $v) {
                $array[$i][$k] = $v;
            }
            $i++;
        }
        $array = (!isset($array[1]) ? $array[0] : $array);
        return ($as_object == true ? (object) $array : $array);
}
share|improve this question

1 Answer 1

up vote 3 down vote

I'd imagine a $stmt->fetchAll() would be more performant than the while-loop and $stmt->fetch(). Rather than paging over an entire set of records one at a time, try returning the results from the database in-bulk. Your code is making a trip to the database with each iteration of your while loop. Your code will still look virtually identical:

$i = 0;
$r = $stmt->fetchAll();
foreach ($r as $data) {
    $array[$i] = array();
    foreach ($data as $k => $v) {
        $array[$i][$k] = $v;
    }
    $i++;
}

If you're using mysqli (native driver only), and not PDO's MySQL, you could do this:

$i = 0;
$r = $stmt->get_result();
$r = $r->fetchAll();
foreach ($r as $data) {
    $array[$i] = array();
    foreach ($data as $k => $v) {
        $array[$i][$k] = $v;
    }
    $i++;
}

You shouldn't worry about loops per se, but performance hits that you see, whether they come from loops or not. This is only possible if you're profiling your code.

share|improve this answer
    
Thanks Quentin, much more explicit. – John Syrinek May 15 '13 at 14:42
    
Yep, thanks, but this doesn't work for be because I use mysqli and the mysqli_stmt object doesn't have a method fetchAll. – sinaneker May 15 '13 at 18:38
    
Sorry user1232375, I didn't realize you're using mysqli. I added some code that should work as long as you're using a native mysqli driver. It uses the mysqli_stmt::get_result() method: php.net/manual/en/mysqli-stmt.get-result.php – John Syrinek May 16 '13 at 21:19
    
Yep, tested it with mysql and it worked! Thank you for your work. – sinaneker May 21 '13 at 9:31
    
Awesome, glad I could help! Would you mind accepting my answer? – John Syrinek May 22 '13 at 17:48

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.