Conversation
ADD AuthServlet, login.jsp, UserRepository
…java ADD prologue.jsp
demologin
left a comment
demologin
left a comment
There was a problem hiding this comment.
Общий вывод по проекту
Проект демонстрирует хорошее понимание основ работы с Java Servlets и сессиями. Успешно реализовал дерево решений квеста и механизм переходов между состояниями. Однако архитектура сильно перегружена логикой в слое контроллеров (сервлетов), что затрудняет поддержку и расширение.
Основные рекомендации:
Выделение бизнес-логики: Перенесите управление состоянием квеста и поиск данных из сервлетов в сервисный слой (QuestService).
Использование фильтров: Вынесите проверку сессии пользователя в Filter, чтобы не дублировать код в каждом сервлете.
Уход от "магических чисел": Используйте Enum для статусов игры и константы для ключей сессии.
Логирование: Замените отсутствие уведомлений или потенциальные System.out.println на полноценный логгер (например, SLF4J).
Проект выглядит крепким для начального уровня, но применение принципов SOLID и паттернов проектирования сделает его профессиональным.
Итоговая оценка: A
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class Quest { |
There was a problem hiding this comment.
Поля 'name' и 'prologue' должны быть final, так как они не меняются после создания объекта. Это обеспечит неизменяемость (immutability). [INFO]
| public class Quest { | ||
| private String name; | ||
| private String prologue; | ||
| private Map<Integer, QuestStep> steps; |
There was a problem hiding this comment.
Рекомендуется использовать интерфейс List или Collection вместо конкретной реализации HashMap в объявлении типа поля, если порядок не важен, или Map, если важен доступ по ID. [INFO]
| public class QuestStep { | ||
| private int id; | ||
| private String text; | ||
| private Map<String, Integer> options; |
There was a problem hiding this comment.
Использование Map<String, Integer> для опций связывает логику переходов с текстом кнопок. Лучше создать отдельный класс Option с полями text и nextStepId. [WARNING]
| private boolean isEnd; | ||
| private boolean isWin; | ||
|
|
||
| public QuestStep(int id, String text, Map<String, Integer> options, boolean isEnd, boolean isWin) { |
There was a problem hiding this comment.
Конструктор принимает слишком много параметров. Стоит рассмотреть паттерн Builder для более удобного создания объектов QuestStep. [INFO]
|
|
||
| @WebServlet("/restart") | ||
| public class RestartServlet extends HttpServlet { | ||
| protected void doGet(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { |
There was a problem hiding this comment.
Отсутствует логирование процесса перезапуска. Рекомендуется использовать SLF4J/Logback вместо пустых строк или отсутствия уведомлений о действиях пользователя. [WARNING]
| import java.io.IOException; | ||
|
|
||
| @WebServlet("/restart") | ||
| public class RestartServlet extends HttpServlet { |
There was a problem hiding this comment.
Класс не содержит комментариев Javadoc, описывающих его назначение. [INFO]
| } | ||
|
|
||
| protected void doPost(HttpServletRequest request, HttpServletResponse response) throws ServletException, IOException { | ||
| String choice = request.getParameter("choice"); |
There was a problem hiding this comment.
Параметр 'choice' из запроса никак не фильтруется, что потенциально может привести к XSS, если эти данные будут выведены обратно без экранирования. [WARNING]
| @@ -0,0 +1,225 @@ | |||
| package com.javarush.toporov.quest.util; | |||
There was a problem hiding this comment.
Класс утилит должен иметь приватный конструктор, чтобы предотвратить создание экземпляров. [INFO]
| import org.junit.jupiter.api.Test; | ||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| class QuestTest { |
There was a problem hiding this comment.
Имя тестового класса QuestTest хорошее, но стоит добавить тесты на негативные сценарии (например, несуществующий ID шага). [INFO]
| import java.io.IOException; | ||
|
|
||
| @WebServlet("/start") | ||
| public class StartServlet extends HttpServlet { |
There was a problem hiding this comment.
Название сервлета StartServlet корректно, но стоит придерживаться единого стиля именования URL (кебаб-кейс или кэмел-кейс). [INFO]
2 participants
No description provided.