Как правильно написать метод модели с динамическими полями?

У пользователя есть 10 полей.
С фронта приходит от 1 до 5 полей, их нужно изменить в БД, не хочется писать под каждое поле отдельный метод в модели и вызывать поочередно.

Накидал такое решение +/-, можно ли как-то его упростить и, может быть, я что-то не учел в нем?
Возможно, есть какие-то ф-ции, которые можно использовать вместо цикла? (не нашел)
public function updateUser($params) {
		$fields = '';

		foreach ($params as $key => $value) {
			if ($key !== 'user_id') {
				$fields .= $key . ' = :' . $key . (end($params) !== $value ? ', ' : '');
			}
		}

		$this->db->query('UPDATE users SET ' . $fields . ' WHERE user_id = :user_id', $params);
	}
  • Вопрос задан
  • 284 просмотра
Решения вопроса 1
FanatPHP
@FanatPHP
Чебуратор тега РНР
Вопрос очень хороший. И код в целом неплохой. А ошибки совершенно стандартные, их делают все.

Начнем с того, что это никакая не модель.
Модель - это вся бизнес-логика приложения.
А это примитивный класс для манипуляции одной таблицей в базе данных. Подходит под паттерн TableGateway

Причем никакие преимущества ООП в нем не используются.
Этот класс - тупо набор функций. Причем для следующей таблицы придется писать точно такой же.

Соответственно, сначала надо сделать код универсальным, и вынести в абстрактный класс.
Заодно исправив в нём одну критическую ошибку (с безопасностью) и одну логическую.
Что будет, если значения двух последних элементов массива совпадут? Тогда уж надо ключи сравнивать, а не значения.
И зачем вообще такие ухищрения с поиском последнего элемента, если у тебя есть переменная $fields? Если она пустая - значит это первый оборот цикла. Всё, этого достаточно

По поводу безопасности я уже устал объяснять.
Мы старательно защищаем значения, передаваемые в запрос, но при этом не моргнув глазом пропускаем инъекцию через имена полей.
Список полей всегда надо писать явно. Тем более что это потом пригодится в миллионе случаев.

abstract class AbstractTableGateway
{
    protected $db;
    protected $table;
    protected $fields;
    protected $primary = 'id';

    public function __construct(DB $db)
    {
        $this->db = $db;
    }
    public function update(array $params): void
    {
        $this->validate($params);

        $set = "";
        foreach($params as $key => $value)
        {
            if ($key !== $this->primary)
                $set .= ($set ? "," : "") . "`$key` = :$key";
            }
        }
        $sql = "UPDATE `$this->table` SET $set WHERE `$this->primary`=:$this->primary";
        $this->db->query($sql, $params);
    }
    protected function validate($data)
    {
        $diff = array_diff(array_keys($data), $this->fields);
        if ($diff) {
            throw new \InvalidArgumentException("Unknown field(s): ". implode(",",$diff));
        }
    }
}


Но вообще я не рекомендую использовать именованные плейсхолдеры для автоматических запросов. В имени поля могут встречаться символы (например пробелы), которые не годятся для имен плейсхолдеров

После этого уже можно создавать класс юзер (и не повторять, как попугай, user user по 10 раз)

class UserGateway extends AbstractTableGateway {
    protected $table = 'users';
    protected $fields = ['email', 'password', 'name', 'birthday'];
    protected $primary = 'user_id';
}


И спокойно к нему обращаться в коде

$user = new UserGateway();
$user->update($params);
Ответ написан
Пригласить эксперта
Ваш ответ на вопрос

Войдите, чтобы написать ответ

Войти через центр авторизации
Похожие вопросы