Explore our sites

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 0 down vote favorite
1

More than likely, the query isn't built correctly as I am fairly green with SQL.

<?
        $i=0;
        foreach($AddedProducts as $AddedProduct)

        {
            $i++;
            echo "<hr>" . $i . "<hr>";
            $query='INSERT INTO products VALUES(';
            foreach($AddedProduct as $key=>$value)
            {
                $query.=$value. (($key=='Cutsheet_Name')? ') ':', ');
                echo $key . ' = ' . $value . (($key=='Picture_Name')? '.jpg<br>':'<br>');
            }
            $query .= 'ON DUPLICATE KEY UPDATE';
        }
        $statement = $db->prepare($query);
        $statement->execute($query);
?>

Edit: Corrected Version in Progress

    <?
        $i=0;
        foreach($AddedProducts as $AddedProduct)
        {
            $arr=Array();
            $i++;
            echo "<hr>" . $i . "<hr>";
            foreach($AddedProduct as $key=>$value)
            {
                $arr[]=$value;
                echo $key . ' = ' . $value . '<br>';
            }
            $query='INSERT INTO products VALUES(' . implode(', ', $arr) . ')';
            $query .= ' ON DUPLICATE KEY UPDATE ';
            foreach($AddedProduct as $key=>$value)
            {
                $query .= $key . '=' . $value . (($key !='Cutsheet_Name')? ', ':';');
            }
            $statement = $db->prepare($query);
            $statement->execute($query);
        }
    ?>
share|improve this question
 
Anyone got any further help on how ON DUPLICATE KEY UPDATE should work? What's the easiest way to have it add 26 values all at once to make new rows? –  Garet Claborn Jun 29 '11 at 14:16
add comment

closed as off-topic by Malachi, retailcoder, syb0rg, Jeff Vanzella, Jamal♦ Dec 17 '13 at 17:04

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Your question must contain working code for us to review it here. For questions regarding specific problems encountered while coding, try Stack Overflow. After getting your code to work, you may edit this question seeking a review of your working code." – Malachi, retailcoder, syb0rg, Jeff Vanzella, Jamal
If this question can be reworded to fit the rules in the help center, please edit the question.

1 Answer

up vote 1 down vote

It might be just me, but it appears this code could use prepared statements to make it safer. Instead of directly appending values to your INSERT, send in a ? and put what's going to replace it in an array.

In addition to that, it looks like you hard-code what looks like the last value of your array so you can build a valid SQL statement. Instead, you should implode(', ', $an_array).

Also, it is generally considered a bad practice to not specify which fields you're actually referencing in your insert. If someone decides to add in an optional 'Comment' field in your products table then this code is going to break unnecessarily.

It's also worth noting that the ON DUPLICATE KEY UPDATE part looks like its missing what it needs to actually update if there is a duplicate.

Finally, you overwrite $query for every added product in your loop, then only execute the last one. This probably isn't what you intended to do.

share|improve this answer
 
Oh man, what a mess lol, thank you. What should ON DUPLICATE KEY UPDATE look like? As to adding new fields, this particular part of the app wont ever be exposed to users aside from myself and uses a static format. Even then I just need to add a new property to the class in order to update interaction with table structure. –  Garet Claborn Jun 28 '11 at 20:56
 
I'm trying to add a new row for any new products which each have a unique key Model_Number but update the row if product is already in the table. –  Garet Claborn Jun 28 '11 at 21:02
add comment

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