Nikolino
@Nikolino

В чем недостатки такого решения?

На техническом собеседовании задали вопрос, что плохого в этом решении.
CREATE TABLE `comments`
( 
  `id` INT(11) NOT NULL AUTO_INCREMENT
  `thread_id` INT(11) NOT NULL,
  `text` VARCHAR(255) NOT NULL, 
  `votes` INT(11) NULL DEFAULT 0,
  PRIMARY KEY(`id`)   
)

public function vote(int $id)
{  
  $repository = App::get('comment'); 
  $comment = $repository->getById($id); 
  ++$comment->votes;   
  $repository->save($comment);  
}


Не смог ответить.
Собеседующий намекнул про одновременные запросы.

Подозреваю, что дело в том, что при одновременных запросах есть шанс того, что из базы прочитается одно и то же значение votes, инкрементируется и запишется в базу. То есть мы потеряем один голос.

С блокировками и транзакциями на практике еще не сталкивался. Думаю, что в эту сторону нужно копать.
Как правильно решать такие задачи, когда нам что-то нужно инкрементировать?
  • Вопрос задан
  • 234 просмотра
Решения вопроса 1
SerafimArts
@SerafimArts
Senior Notepad Reader
В чем недостатки такого решения?

Помимо отсутствия транзакционности в коде (о чём уже упоминали выше):

1) Нет ключа на thread_id (желательно foreign)
2) Типы в INT, вместо UNSIGNED INT
Т.к. это MySQL, судя по вопросу, то:
3) Нет указания на collation таблицы (желательно utf8mb4)
4) Нет указания на движок таблицы (транзакции есть только в InnoDB и их нет в ISAM, MyISAM, Archive, Memory, etc.)
5) Нет уникальности голосов (т.е. votes), как следствие - таблица денормализованная и подсчитывать так голоса - крайне глупо. В нормальном мире это делается отдельной таблицей с привязкой по comment_id и уникальным ключом на comment_id + user_id.
6) Так как это комментарии, то использование varchar(255) для поля text - глупо. Нужен TEXT или LONGTEXT.
7) Отсутствуют поля даты создания и редактирования, что в будущем не позволит строить статистические данные или расширять функционал (разрешать менять текст комментария только в течении 5ти минут, например). На это надо закладываться заранее.

Это что касается структуры таблицы. Теперь по коду:
8) Сервис-локация (класс App), вместо нормального DI - это жесть. Скрытые зависимости говорят о невозможности тестирования такого кода.
9) "Магическая" константа 'comment' говорит о том, что потом такой код нормально не отрефакторить. Т.е. переименовываешь внутри что-то, а потом получаешь кучу ошибок и придётся лазать по коду и везде выгребать этот "get('comment')". И не дай бог где-то они получаются как-то иначе. Работы ещё на пол дня.
10) Метод getById говорит о том, что сущность должна возвращаться всегда. Однако, может так случиться, что её по указанному id просто не найдётся. Как следствие - нет нормальной обработки ошибок.
11) Мутабельное поле votes у comment (т.е. имеется прямой доступ к нему). Это говорит о том, что сущность очень сильно связана с БД, а значит любой рефакторинг таблицы ведёт к полному перелопачиванию всего кода.
12) Нет обработки ошибок во время сохранения сущности.
13) Метод vote - не лучшее название. Оно переводится и как глагол "голосовать" и как существительное "голос", что может запутать. Я бы переименовал в addVote. Хотя и так тоже норм.

Это всё, что касается вопроса "в чём недостатки такого решения". Ответ - почти на каждой строчке косяк.

Если есть вопросы - задавай, могу чуть поподробнее расписать что-либо.
Ответ написан
Пригласить эксперта
Ответы на вопрос 1
@dimoff66
Кратко о себе: Я есть
Вообще если идет голосование неплохо бы определять ip голосовавшего, если он не авторизован или user_id, иначе потеря одного голоса будет мелочью по сравнению с тем, что один человек может голосовать несчетное кол-во раз. И если надо запомнить голосовавшего, значит нужна отдельная таблица с полями voter и comment_id
Ответ написан
Ваш ответ на вопрос

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

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