Review 30.06:
Проект большой, буду создавать отдельные тикеты для разных компонент. В этом ревью рассматриваю room.js.
- Мне кажется, что можно было не заводить на каждое поле, которое нужно передать в js-скрипт, отдельный json. Передайте просто один словарь
data
со всеми параметрами и десериализуйте его на клиенте. А если ещё jquery подключить, то код будет выглядеть намного лаконичнее, примеры ниже. https://mks2.cs.msu.ru/-/ide/project/Sokomi/website_project/tree/main/-/lozhkosson/static/js/room.js/#L1
let data = JSON.parse($("#data").text());
let online_users_selector = $("online_users_selector");
// да на самом деле даже переменную уже не нужно будет заводить,
// просто в коде пишете $("online_users_selector"), где этот элемент используется
- Выберите стиль кодирования, который используйте в проекте, чтобы оформление всех скриптов (а тем более внутри одного скрипта) было одинаковое. Сейчас переменные и функции называются как попало:
roomSocket
,online_Users_Selector
,turn_count
. Например, можете придерживаться Google Style Guide: https://google.github.io/styleguide/jsguide.html . Но вариантonline_Users_Selector
в любом известном гайде будет не ок. - Ниже идут у вас три функции с дублированием кода, которые добавляют опцию в селектор. Можно сделать одну функцию с параметрами. А ещё посмотрите, насколько понятнее выглядит то же самое в jquery: https://stackoverflow.com/questions/740195/adding-options-to-a-select-using-jquery
- Эта и функция выше делает одно и то же. Да и функция ниже примерно то же самое. https://mks2.cs.msu.ru/-/ide/project/Sokomi/website_project/tree/main/-/lozhkosson/static/js/room.js/#L102
- Зачем мы вообще используем option в div элементе? Он же предназначатся для использования в select и внутри некоторых других тегов. https://developer.mozilla.org/en-US/docs/Web/HTML/Element/option
- Все проверки должны быть на бекенде. Их можно дублировать на клиенте, но это делается для удобства, чтобы не делать лишних запросов в бекенд для валидации. https://mks2.cs.msu.ru/-/ide/project/Sokomi/website_project/tree/main/-/lozhkosson/static/js/room.js/#L122
- Валидации лучше оформлять не цепочкой вложенных ифов, а примерно так:
if (username !== creator) {
alert(...);
return;
}
if (online_users_count !== players_limit) {
alert(...);
return;
}
... тут основная логика метода ...
- Зачем удалять элемент, если можно просто поменять значение? Заведите один элемент (и не option, а обычный div, span или ещё что-нибудь подходящее) с айдишником, например, "online_users_count". Дальше в скриптах после изменения этого числа будете делать просто
$("#online_users_count").html(online_users_count)
. https://mks2.cs.msu.ru/-/ide/project/Sokomi/website_project/tree/main/-/lozhkosson/static/js/room.js/#L193 - В jquery будут методы
hide
иshow
. https://mks2.cs.msu.ru/-/ide/project/Sokomi/website_project/tree/main/-/lozhkosson/static/js/room.js/#L227
Там на самом деле много ещё что можно порефакторить, но каждый момент описывать это долго. После перехода на jquery и устранения дублирования должно стать заметно меньше кода и получиться более читаемо.