current community

your communities

more stack exchange communities

Stack Exchange
tour help
Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 1 down vote favorite

This is a INSERT function. It's working... I just pulled out the function from the class, atm to try it out. And improve. I think the code is a bit messy, And I am pretty sure I can improve the code, less code, for example.. Any ideas?

   $data['test'] = array('username' => 'john', 
              'password' => 'hello',
              'userlevel' => '__d');

$table = 'users';

$numItems = count($data['test']);
$i = 0;

$sql = "INSERT INTO " . $table . "(". implode(", ", array_keys($data['test'])) .")";


$sql .= " VALUES (";

$i = 0;

foreach ($data['test'] as $value) {

    if ($i+1 == $numItems and $value == '__d') {
        $sql .= "" . 'NOW()' . ")";
    } else if ($i+1 == $numItems) {
        $sql .= "'" . $value . "')";
    } else if ($value == '__d') {
        $sql .= "" . 'NOW()' . ", ";
    } else {
        $sql .= "'" . $value . "', ";
    }

            $i++;


}

echo $sql;
share|improve this question
    
inside the foreach why the comparation of $i + 1 == $numItems if you end up doing the same, be that comparation true or false? –  Carlos Campderrós Sep 28 '11 at 14:38
    
Well I need to determine If I am going to end with a , or ') :) –  John Sep 28 '11 at 14:43

2 Answers 2

up vote 2 down vote

This is what you have, cleaned a little

foreach ($data['test'] as $value) {

    if ($value == '__d') {
        $sql .= 'NOW()';
    } else {
        $sql .= "'" . $value . "'";
    }
    $i++; // add before the comparisson
    // make the comparrison at one place
    // if the number != total, then we need a comma.
    if($i != $numItems) $sql .= ',';
}
// always end with a close paren.
$sql .= ')';

But you are better off using implode and replace:

// implode using the quotes and commas.
$values = "'".implode("', '", array_values($values))."'";
// swap out __d for NOW
$sql .= str_replace("'__d'",'NOW()',$values);

Of course, you really need to make sure that you've cleaned all of those values with mysql_real_escape_string.

share|improve this answer
up vote 0 down vote

Why do you not do:

VALUES = implode(", ", array_values($values))

If you can do:

$values = array('username' => "'john'", 
            'password' => "'hello'",
            'userlevel' => "0");

There are many way to mark up the value of array with double quote.

share|improve this answer
    
It feels easier to type 'username' => $_POST['username'], 'date' = '__d' then just having all these "" and '' –  John Sep 28 '11 at 14:39

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.