Alimov by Phoenix190696 · Pull Request #4 · demologin/ProjectPantera

Phoenix190696 · 2026-01-24T10:48:30Z

No description provided.

demologin · 2026-01-31T11:07:00Z

Общий вывод по проекту

Проект представляет собой классическую реализацию веб-квеста на базе Java Servlets. Автор успешно применил Mockito для тестирования контроллеров и продемонстрировал понимание жизненного цикла HTTP-сессий. Структура проекта логична, а именование классов и переменных в целом соответствует стандартам Java.

Однако, в архитектуре наблюдается сильная связанность (tight coupling) из-за использования статических методов и прямой работы с состоянием в сервлете. Внедрение паттерна "Состояние" и выделение сервисного слоя значительно повысят гибкость и расширяемость приложения. Также стоит уделить внимание безопасности (валидация ввода) и современным практикам логирования.

Итоговая оценка: A (online review)

demologin · 2026-01-31T11:07:01Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+
+        HttpSession session = req.getSession();
+        GameState state = (GameState) session.getAttribute("state");
+        if (state == null) state = GameState.START;


Использование жестко закодированного состояния по умолчанию напрямую в контроллере нарушает инкапсуляцию. Рекомендуется вынести логику инициализации в GameLogic.

demologin · 2026-01-31T11:07:02Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+        if (state == null) state = GameState.START;
+
+        String answer = req.getParameter("answer");
+        GameResult result = GameLogic.next(state, answer);


Контроллер напрямую вызывает статический метод GameLogic.next. Это затрудняет тестирование и нарушает принцип инверсии зависимостей. Стоит использовать внедрение зависимостей.

demologin · 2026-01-31T11:07:03Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+
+        if (result.state() == GameState.WIN || result.state() == GameState.LOSE) {
+            Integer gamesPlayed = (Integer) session.getAttribute("gamesPlayed");
+            session.setAttribute("gamesPlayed", gamesPlayed == null ? 1 : gamesPlayed + 1);


Логика инкремента счетчика игр смешана с логикой обработки HTTP-запроса. Данную операцию следует вынести в отдельный сервисный слой.

demologin · 2026-01-31T11:07:04Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+        if (result.state() == GameState.WIN || result.state() == GameState.LOSE) {
+            Integer gamesPlayed = (Integer) session.getAttribute("gamesPlayed");
+            session.setAttribute("gamesPlayed", gamesPlayed == null ? 1 : gamesPlayed + 1);
+            req.getRequestDispatcher("/WEB-INF/result.jsp").forward(req, resp);


Пути к JSP ресурсам ('/WEB-INF/game.jsp') дублируются в коде. Целесообразно определить их как константы, чтобы избежать ошибок при опечатках.

demologin · 2026-01-31T11:07:04Z

src/main/java/com/javarush/alimov/quest/GameLogic.java

+
+public class GameLogic {
+
+    public static GameResult next(GameState current, String answer) {


Метод next() содержит сложную структуру switch-case, управляющую переходами состояний. Это классическое нарушение принципа открытости/закрытости (OCP). Рекомендуется использовать паттерн State.

demologin · 2026-01-31T11:07:05Z

src/main/java/com/javarush/alimov/quest/GameLogic.java

+            case START -> new GameResult(GameState.UFO_CHALLENGE, null);
+
+            case UFO_CHALLENGE -> {
+                if ("accept".equals(answer)) {


Строковые литералы для ответов (например, 'accept', 'go') разбросаны по всему коду. Рекомендуется использовать перечисления (Enum) для возможных действий игрока.

demologin · 2026-01-31T11:07:06Z

src/main/java/com/javarush/alimov/quest/GameLogic.java

+                } else {
+                    yield new GameResult(GameState.LOSE, "Ты отклонил вызов. Поражение.");
+                }
+            }


Текстовые сообщения для пользователя жестко прописаны в логике. Для поддержки интернационализации (i18n) их следует вынести в ResourceBundle.

demologin · 2026-01-31T11:07:07Z

src/main/java/com/javarush/alimov/quest/GameState.java

@@ -0,0 +1,10 @@
+package com.javarush.alimov.quest;


Перечисление GameState содержит как промежуточные состояния, так и терминальные (WIN, LOSE). Стоит рассмотреть разделение логики навигации и статуса завершения.

demologin · 2026-01-31T11:07:09Z

src/test/java/com/javarush/alimov/quest/GameLogicTest.java

+class GameLogicTest {
+
+    @Test
+    void testVictoryPath() {


Тест 'testVictoryPath' проверяет сразу всю цепочку игры. В юнит-тестах предпочтительнее проверять каждый переход состояния изолированно.

demologin · 2026-01-31T11:07:10Z

src/test/java/com/javarush/alimov/quest/GameLogicTest.java

+        result = GameLogic.next(result.state(), "truth");
+
+        assertEquals(GameState.WIN, result.state());
+        assertEquals("Тебя вернули домой. Победа!", result.message());


Проверка текстовых сообщений в тестах делает их хрупкими. При изменении формулировки сообщения тесты упадут, хотя логика останется верной.

demologin · 2026-01-31T11:07:11Z

src/test/java/com/javarush/alimov/quest/GameServletTest.java

+
+    @BeforeEach
+    void setUp() {
+        MockitoAnnotations.openMocks(this);


Использование MockitoAnnotations.openMocks(this) считается устаревшим. Рекомендуется использовать аннотацию @ExtendWith(MockitoExtension.class).

demologin · 2026-01-31T11:07:11Z

src/test/java/com/javarush/alimov/quest/GameServletTest.java

+
+    @Test
+    void doPost_StartToUfoChallenge() throws ServletException, IOException {
+        when(session.getAttribute("state")).thenReturn(null);


Тест завязан на внутреннее поведение сессии. Лучше тестировать контракт взаимодействия через сервис, скрывающий детали работы с HttpSession.

demologin · 2026-01-31T11:07:12Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+
+import jakarta.servlet.http.*;
+import jakarta.servlet.annotation.*;
+import jakarta.servlet.*;


Использование wildcard import (jakarta.servlet.http.*) не рекомендуется, так как это затрудняет понимание используемых зависимостей.

demologin · 2026-01-31T11:07:13Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+    protected void doPost(HttpServletRequest req, HttpServletResponse resp)
+            throws ServletException, IOException {
+
+        HttpSession session = req.getSession();


Отсутствует логирование действий пользователя или ошибок. Использование SLF4J помогло бы в диагностике проблем на сервере.

demologin · 2026-01-31T11:07:13Z

src/main/java/com/javarush/alimov/quest/GameLogic.java

@@ -0,0 +1,37 @@
+package com.javarush.alimov.quest;
+


Класс GameLogic не имеет приватного конструктора, хотя содержит только статические методы. Следует предотвратить создание экземпляров утилитного класса.

demologin · 2026-01-31T11:07:14Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+        GameState state = (GameState) session.getAttribute("state");
+        if (state == null) state = GameState.START;
+
+        String answer = req.getParameter("answer");


Параметр 'answer' получается напрямую из запроса без валидации. Это потенциальная уязвимость, если данные будут использоваться в выводе без экранирования.

demologin · 2026-01-31T11:07:15Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+        session.setAttribute("message", result.message());
+
+        if (result.state() == GameState.WIN || result.state() == GameState.LOSE) {
+            Integer gamesPlayed = (Integer) session.getAttribute("gamesPlayed");


Счетчик игр хранится в сессии как Integer. При конкурентных запросах (хотя здесь это маловероятно) возможны проблемы. Стоит рассмотреть атомарные типы или сервисный слой.

demologin · 2026-01-31T11:07:15Z

src/test/java/com/javarush/alimov/quest/GameServletTest.java

+
+import java.io.IOException;
+
+import static org.mockito.Mockito.*;


Статические импорты Mockito (when, verify) — это хорошая практика, стоит продолжать их использовать для чистоты тестов.

demologin · 2026-01-31T11:07:17Z

src/main/java/com/javarush/alimov/quest/GameServlet.java

+import java.io.IOException;
+
+@WebServlet("/game")
+public class GameServlet extends HttpServlet {


Методы doGet и doPost содержат дублирующийся код вызова Dispatcher. Общую логику перехода можно вынести в защищенный метод.

demologin · 2026-01-31T11:07:17Z

src/main/java/com/javarush/alimov/quest/GameState.java

+
+public enum GameState {
+    START,
+    UFO_CHALLENGE,


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

demologin · 2026-01-31T11:07:19Z

src/test/java/com/javarush/alimov/quest/GameLogicTest.java

+
+import static org.junit.jupiter.api.Assertions.*;
+
+class GameLogicTest {


Класс теста не имеет модификатора доступа public, что хорошо (пакетная видимость достаточна для JUnit 5).

Phoenix190696 added 3 commits January 18, 2026 12:57

ADD LOGIC,JSP

74600ee

Complete

c62e582

Complete

d545a1e

demologin reviewed Jan 31, 2026

View reviewed changes


		public class GameLogic {

		public static GameResult next(GameState current, String answer) {


		import java.io.IOException;

		import static org.mockito.Mockito.*;


		import static org.junit.jupiter.api.Assertions.*;

		class GameLogicTest {

Conversation

Phoenix190696 commented Jan 24, 2026

Uh oh!

demologin commented Jan 31, 2026

Общий вывод по проекту

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants