2

本を読んでいるときに、次の機能に出くわしました。

/*
Update records in the database
@param String $table the table being updated
@param Array $changes array of changes field => value
@param String $condition the condition
@return Boolean
*/
public function updateRecords($table, array $changes, $condition)
{
    $update = "UPDATE " . $table . " SET ";
    foreach($changes as $field => $value)
    {
        $update .= "`" . $field . "` = '{$value}', ";
    }
    //remove trailing , (comma)
    $update .= substr($update, 0, -1);

    if($condition != '')
    {
        $update .= "WHERE " . $condition;
    }
    $this->executeQuery($update);
    //Not sure why it returns true.
    return true;
}

私が間違っている場合は訂正してください。ただし、これはデータのフィルタリング/チェックがまったくない、設計が不適切な関数ではありません。そして何よりも、この関数は常に「true」を返します。

4

1 に答える 1

0

正しい。サニテーションを一切行わずに文字列連結によるSQL文を構築しているようです。すべての入力が信頼できる場合、これは許容されるかもしれませんpublicが、それが機能であることを考えると、そうではないと思います。

これは、例外を呼び出したりスローしたりするreturn true代わりに、使用するコードがサイレント モードで失敗することを意味します。die()

.スタイルに関しては、著者は連結と一貫性がありません。ある場所では演算子を使用した単純な連結であり、他の場所では を使用しています$placeholders。

が空の場合$changes、 の呼び出しsubstrは失敗します。リストを作成した後にトリムを行うのは、リスト項目をコンマで区切るのに間違った方法です。これは私がそれを行う方法です（もちろん、決してSQLを使用しないことを除いて）：

$first = true;
foreach($changes as $field => $value ) {
    if( !$first ) $update .= ", ";
    $first = false;
    $update .= "`$field` = '{$value}'"; // NEVER DO THIS
}

考えてみると、この機能の唯一の利点は、十分に文書化されていることです。

于 2013-08-06T02:35:25.390 に答える