Skip to content

Add test, log MARTYNOV Pantera#6

Open
ViktorMartynovJava wants to merge 1 commit intodemologin:mainfrom
ViktorMartynovJava:martynov
Open

Add test, log MARTYNOV Pantera#6
ViktorMartynovJava wants to merge 1 commit intodemologin:mainfrom
ViktorMartynovJava:martynov

Conversation

@ViktorMartynovJava
Copy link

Change pom.xml

Change pom.xml
Copy link
Owner

@demologin demologin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Проект представляет собой добротную базу для веб-приложения на сервлетах. Основные сильные стороны: использование современного стека (Java 21, JUnit 5, Mockito, SLF4J), наличие логирования и попытка реализовать глобальную статистику через ServletContext.

Основные зоны роста:

Архитектура: Сервлет сейчас выполняет роль «божественного объекта» (God Object). Вынесение логики квеста в сервисный слой сделает код более тестируемым и чистым.

Масштабируемость: Использование switch-case для шагов квеста быстро станет проблемой. Изучение паттернов проектирования (State, Strategy) поможет сделать структуру гибкой.

Тестирование: Тесты написаны грамотно, но стоит уделить внимание расположению пакетов.

Это отличный старт! Видно, что вы уделяете внимание деталям и стремитесь использовать профессиональные инструменты. Продолжайте изучать SOLID и паттерны проектирования — они помогут вам превратить этот проект в по-настоящему гибкую и мощную систему. Удачи в изучении Java!

Итоговая оценка: B

import java.util.concurrent.ConcurrentHashMap;

@WebServlet(name = "QuestServlet", value = "/quest")
public class QuestServlet extends HttpServlet {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Класс нарушает принцип единственной ответственности (SRP). Он управляет сессиями, логикой игры и статистикой. Рекомендуется выделить бизнес-логику в класс GameService. [ERROR]

int currentStep = state.getStep();
String name = state.getPlayerName();

switch (currentStep) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Использование switch-case для описания шагов квеста делает код 'жестким'. Рекомендуется использовать паттерн 'State' или загрузку сценария из внешнего JSON-файла. [WARNING]

@Override
public void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException {
HttpSession session = req.getSession();
GameState state = (GameState) session.getAttribute("state");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Потенциальный NullPointerException: извлечение атрибута 'state' из сессии без проверки на null. При истечении времени сессии это приведет к падению приложения. [ERROR]

state.incrementGames();
}

req.getRequestDispatcher("/quest.jsp").forward(req, resp);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Путь к JSP-файлу '/quest.jsp' захардкожен. Целесообразно вынести пути к представлениям в константы для упрощения рефакторинга. [INFO]

}

ServletContext context = getServletContext();
Map<String, UserStats> statsMap = (Map<String, UserStats>) context.getAttribute("globalStats");
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Хранение глобальной статистики в ServletContext (statsMap) не надежно. Данные будут утеряны при перезагрузке сервера. Стоит рассмотреть использование БД. [WARNING]

import jakarta.servlet.ServletContext;
import jakarta.servlet.ServletException;
import jakarta.servlet.annotation.WebServlet;
import jakarta.servlet.http.HttpServlet;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сервлет не хранит состояние в полях класса, что делает его безопасным для использования в многопоточной среде Java EE. [INFO]

this.deathReason = deathReason;
}

public void resetGame() {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Метод resetGame корректно сбрасывает текущий шаг, но сохраняет накопленную статистику побед, что удобно для игрока. [INFO]

} else if ("answer".equals(action)) {
String choice = req.getParameter("choice");
logger.debug("Player '{}' made a choice: {} on step {}", state.getPlayerName(), choice, state.getStep());
processGameStep(state, choice);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Прямое управление полем step объекта GameState из сервлета нарушает инкапсуляцию. Лучше реализовать метод state.moveToNextStep(). [WARNING]

import org.junit.jupiter.api.extension.ExtendWith;
import org.mockito.Mock;
import org.mockito.InjectMocks;
import org.mockito.junit.jupiter.MockitoExtension;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Использование Mockito для имитации объектов контейнера сервлетов — правильный путь для изоляции тестируемого компонента. [INFO]


assertEquals(2, state.getStep(), "После выбора 'trust' на 1 шаге, текущий шаг должен быть 2");

verify(dispatcher, times(1)).forward(request, response);
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тест проверяет вызов forward, что гарантирует правильное завершение жизненного цикла запроса в сервлете. [INFO]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants