PHP - SQL Statement created wrong-CodePudding

I try to implement a view counter with database for my website.

My code:

Function, that creates the query:

public function addRowViewCompany($arrElement) {
        $query = "INSERT INTO views_company (";
        $columns = "";
        $values = "";

        // columns
        foreach ($arrElement as $column => $value) {

            if ($value === end($arrElement)) {
                // if last column
                $columns .= $column . ") VALUES(";
                $values .= ":" . $column . ");";
            } else {
                $columns .= $column . ", ";
                $values .= ":" . $column . ", ";
            }
        }

        $this->setQuery($query . $columns . $values);
        $this->prepareStatement();
        $this->executeStatement($arrElement);
    }

calling the method:

            $database->addRowViewCompany(array(
                "view_id" => null,
                "view_date" => date("Y-m-d"),
                "company_id" => intval($companyID),
                "views" => 1
            ));

The query that gets created:

INSERT INTO views_company (view_id, view_date, company_id) VALUES(views) VALUES(:view_id, :view_date, :company_id);:views);

The Query is wrong at this location: INSERT INTO views_company (view_id, view_date, company_id) VALUES(views ) VALUES(:view_id, :view_date, :company_id );: views);

But I don't get why it gets created wrong For other use cases this function works.

EDIT:

public function __construct() {
    $this->connection = new PDO("mysql:host=localhost;dbname=ref;charset=utf8", "***", "***");
}

public function setQuery(string $query) {
    $this->query = $query;
}

public function prepareStatement() {
    $this->statement = $this->connection->prepare($this->query);
}

public function executeStatement(array $args = null) {
    if ($args == null) {
        $this->statement->execute();
    } else {
        $this->statement->execute($args);
    }
}

CodePudding user response：

Your code won't work if there's a value that duplicates the last value in the array, because $value === end($arrElement) will be true for the duplicate. That will cause you to add an extra VALUES to the query.

Don't use loops, use implode(). Then you don't need to treat the last iteration specially.

$columns = implode(', ', array_keys($arrElement);
$values = implode(', ', array_map(function($col) { return ':' . $col; }, array_keys($arrElement)));

$this->setQuery("$query ($columns) VALUES ($values)");

CodePudding user response：

What about this modification to your code?

public function addRowViewCompany($arrElement) {
    $query = "INSERT INTO views_company (";
    $columns = "";
    $middle = ") VALUES(";
    $values = "";
    $end = ")";

    // columns
    foreach ($arrElement as $column => $value) {
        $columns .= ((strlen($column)==0)?"":", ") . $column;
        $values .= ((strlen($values)==0)?"":", ") . ":" . $column;
    }

    $this->setQuery($query . $columns . $middle . $values . $end);
    $this->prepareStatement();
    $this->executeStatement($arrElement);
}