-
Notifications
You must be signed in to change notification settings - Fork 27
Add test, log MARTYNOV Pantera #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| package com.javarush.martynov; | ||
|
|
||
| public class GameState { | ||
| private String playerName; | ||
| private int step = 0; | ||
| private int gamesPlayed = 0; | ||
| private String lastMessage; | ||
| private int wins; | ||
| private int losses; | ||
|
|
||
| public int getWins() { | ||
| return wins; | ||
| } | ||
|
|
||
| public void setWins(int wins) { | ||
| this.wins = wins; | ||
| } | ||
|
|
||
| public int getLosses() { | ||
| return losses; | ||
| } | ||
|
|
||
| public void setLosses(int losses) { | ||
| this.losses = losses; | ||
| } | ||
|
|
||
| public GameState() { | ||
| } | ||
|
|
||
| private String deathReason; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Поле deathReason может содержать null. Это может привести к некорректному отображению на фронтенде. Рекомендуется использовать пустую строку по умолчанию. [WARNING] |
||
|
|
||
| public String getDeathReason() { | ||
| return deathReason; | ||
| } | ||
|
|
||
| public void setDeathReason(String deathReason) { | ||
| this.deathReason = deathReason; | ||
| } | ||
|
|
||
| public void resetGame() { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Метод resetGame корректно сбрасывает текущий шаг, но сохраняет накопленную статистику побед, что удобно для игрока. [INFO] |
||
| this.step = 0; | ||
| this.deathReason = ""; | ||
| } | ||
|
|
||
|
|
||
| public String getPlayerName() { | ||
| return playerName; | ||
| } | ||
|
|
||
| public void setPlayerName(String playerName) { | ||
| this.playerName = playerName; | ||
| } | ||
|
|
||
| public int getStep() { | ||
| return step; | ||
| } | ||
|
|
||
| public void setStep(int step) { | ||
| this.step = step; | ||
| } | ||
|
|
||
| public int getGamesPlayed() { | ||
| return gamesPlayed; | ||
| } | ||
|
|
||
| public void incrementGames() { | ||
| this.gamesPlayed++; | ||
| } | ||
|
|
||
| public String getLastMessage() { | ||
| return lastMessage; | ||
| } | ||
|
|
||
| public void setLastMessage(String lastMessage) { | ||
| this.lastMessage = lastMessage; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,152 @@ | ||
| package com.javarush.martynov; | ||
|
|
||
| import jakarta.servlet.ServletContext; | ||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.annotation.WebServlet; | ||
| import jakarta.servlet.http.HttpServlet; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Сервлет не хранит состояние в полях класса, что делает его безопасным для использования в многопоточной среде Java EE. [INFO] |
||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import jakarta.servlet.http.HttpSession; | ||
| import org.slf4j.Logger; | ||
| import org.slf4j.LoggerFactory; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Использование логгера SLF4J вместо System.out.println — признак профессионального подхода. Это позволяет гибко настраивать вывод данных. [INFO] |
||
|
|
||
| import java.io.IOException; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Использование ConcurrentHashMap — отличная практика для многопоточной среды, однако инициализация карты должна быть синхронизирована или вынесена в ServletContextListener. [WARNING] |
||
|
|
||
| @WebServlet(name = "QuestServlet", value = "/quest") | ||
| public class QuestServlet extends HttpServlet { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Класс нарушает принцип единственной ответственности (SRP). Он управляет сессиями, логикой игры и статистикой. Рекомендуется выделить бизнес-логику в класс GameService. [ERROR] |
||
| private static final Logger logger = LoggerFactory.getLogger(QuestServlet.class); | ||
|
|
||
| @Override | ||
| public void doGet(HttpServletRequest req, HttpServletResponse resp) throws ServletException, IOException { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Метод doGet слишком длинный и содержит много вложенных условий. Рекомендуется разделить его на небольшие методы handleStart и handleAction. [WARNING] |
||
| HttpSession session = req.getSession(); | ||
| GameState state = (GameState) session.getAttribute("state"); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Потенциальный NullPointerException: извлечение атрибута 'state' из сессии без проверки на null. При истечении времени сессии это приведет к падению приложения. [ERROR] |
||
| String sessionId = session.getId(); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ручное управление sessionId в логах можно автоматизировать с помощью MDC (Mapped Diagnostic Context) в конфигурации логгера. [INFO] |
||
|
|
||
| if (state == null) { | ||
| state = new GameState(); | ||
| session.setAttribute("state", state); | ||
| logger.info("New game state created for session: {}", sessionId); | ||
| } | ||
|
|
||
| String action = req.getParameter("action"); | ||
| logger.debug("Received action: {} for session: {}", action, sessionId); | ||
|
|
||
| if ("start".equals(action)) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Риск NPE: вызов action.equals("start"). Рекомендуется использовать Yoda-style: "start".equals(action), чтобы обезопасить код от пустых параметров. [INFO] |
||
| String name = req.getParameter("name"); | ||
| state.setPlayerName(name); | ||
| state.setStep(1); | ||
| logger.info("Player '{}' has started the journey (Session: {})", name, sessionId); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Логирование факта старта игры помогает отслеживать вовлеченность пользователей без использования сложных аналитических систем. [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); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Прямое управление полем step объекта GameState из сервлета нарушает инкапсуляцию. Лучше реализовать метод state.moveToNextStep(). [WARNING] |
||
|
|
||
| } else if ("restart".equals(action)) { | ||
| logger.info("Player '{}' requested a restart. Games played before restart: {}", state.getPlayerName(), state.getGamesPlayed()); | ||
| state.resetGame(); | ||
| state.incrementGames(); | ||
| } | ||
|
|
||
| req.getRequestDispatcher("/quest.jsp").forward(req, resp); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Путь к JSP-файлу '/quest.jsp' захардкожен. Целесообразно вынести пути к представлениям в константы для упрощения рефакторинга. [INFO] |
||
| } | ||
|
|
||
| private void updateGlobalStatistics(String name, boolean isWin) { | ||
| if (name == null || name.isEmpty()) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Добавлена валидация на null и пустую строку для имени игрока. Это пример правильного защитного программирования. [INFO] |
||
| logger.warn("Attempted to update statistics for a null or empty player name."); | ||
| return; | ||
| } | ||
|
|
||
| ServletContext context = getServletContext(); | ||
| Map<String, UserStats> statsMap = (Map<String, UserStats>) context.getAttribute("globalStats"); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Хранение глобальной статистики в ServletContext (statsMap) не надежно. Данные будут утеряны при перезагрузке сервера. Стоит рассмотреть использование БД. [WARNING] |
||
|
|
||
| if (statsMap == null) { | ||
| statsMap = new ConcurrentHashMap<>(); | ||
| context.setAttribute("globalStats", statsMap); | ||
| logger.debug("Global statistics map initialized in ServletContext."); | ||
| } | ||
|
|
||
| UserStats userStats = statsMap.computeIfAbsent(name, k -> { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Элегантное использование метода computeIfAbsent для атомарного обновления счетчика побед/поражений. Хорошая работа с API коллекций. [INFO] |
||
| logger.debug("Creating new UserStats entry for player: {}", name); | ||
| return new UserStats(); | ||
| }); | ||
|
|
||
| if (isWin) { | ||
| userStats.addWin(); | ||
| logger.info("Global stats updated: Player '{}' WON.", name); | ||
| } else { | ||
| userStats.addLoss(); | ||
| logger.info("Global stats updated: Player '{}' LOST.", name); | ||
| } | ||
| } | ||
|
|
||
| private void processGameStep(GameState state, String choice) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Метод processGameStep является приватным, что затрудняет его тестирование. Это следствие того, что логика заперта внутри сервлета. [INFO] |
||
| int currentStep = state.getStep(); | ||
| String name = state.getPlayerName(); | ||
|
|
||
| switch (currentStep) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Использование switch-case для описания шагов квеста делает код 'жестким'. Рекомендуется использовать паттерн 'State' или загрузку сценария из внешнего JSON-файла. [WARNING] |
||
| case 1: | ||
| if ("trust".equals(choice)) { | ||
| state.setStep(2); | ||
| } else { | ||
| state.setDeathReason("Вы вышли на открытое пространство и стали легкой добычей для стаи зомби."); | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Текстовые константы (причины поражения) следует вынести в ResourceBundle для поддержки интернационализации (i18n). [WARNING] |
||
| state.setStep(-1); | ||
| logger.warn("Player '{}' died at Step 1. Choice: {}", name, choice); | ||
| updateGlobalStatistics(name, false); | ||
| } | ||
| break; | ||
|
|
||
| case 2: | ||
| if ("truth".equals(choice)) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Отсутствует ветка default в switch. Если состояние квеста станет некорректным, пользователь увидит пустую страницу или ошибку 500. [ERROR] |
||
| state.setStep(3); | ||
| } else { | ||
| state.setDeathReason("Вступать в рукопашную с зомби было плохой идеей. Вас повалили числом."); | ||
| state.setStep(-1); | ||
| logger.warn("Player '{}' died at Step 2. Choice: {}", name, choice); | ||
| updateGlobalStatistics(name, false); | ||
| } | ||
| break; | ||
|
|
||
| case 3: | ||
| if ("truth".equals(choice)) { | ||
| state.setStep(4); | ||
| } else { | ||
| state.setDeathReason("В сумерках вы не заметили ловушку в заброшенном здании и погибли."); | ||
| state.setStep(-1); | ||
| logger.warn("Player '{}' died at Step 3. Choice: {}", name, choice); | ||
| updateGlobalStatistics(name, false); | ||
| } | ||
| break; | ||
|
|
||
| case 4: | ||
| if ("truth".equals(choice)) { | ||
| state.setStep(5); | ||
| } else { | ||
| state.setDeathReason("Охранник почувствовал ложь в вашем голосе. Он решил не рисковать и выстрелил."); | ||
| state.setStep(-1); | ||
| logger.warn("Player '{}' died at Step 4. Choice: {}", name, choice); | ||
| updateGlobalStatistics(name, false); | ||
| } | ||
| break; | ||
|
|
||
| case 5: | ||
| if ("scan".equals(choice)) { | ||
| state.setStep(6); | ||
| logger.info("SUCCESS! Player '{}' reached the bunker.", name); | ||
| updateGlobalStatistics(name, true); | ||
| } else { | ||
| state.setDeathReason("Ваша попытка прорваться силой закончилась быстро. Охранники на вышках не промахиваются."); | ||
| state.setStep(-1); | ||
| logger.warn("Player '{}' died at Step 5 (The Scanner). Choice: {}", name, choice); | ||
| updateGlobalStatistics(name, false); | ||
| } | ||
| break; | ||
|
|
||
| default: | ||
| logger.error("Unexpected game step {} encountered for player '{}'", currentStep, name); | ||
| break; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| package com.javarush.martynov; | ||
|
|
||
| public class UserStats { | ||
|
|
||
| private int wins = 0; | ||
| private int losses = 0; | ||
|
|
||
| public void addWin() { wins++; } | ||
| public void addLoss() { losses++; } | ||
|
|
||
| public int getWins() { return wins; } | ||
| public int getLosses() { return losses; } | ||
| public int getTotal() { return wins + losses; } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| <?xml version="1.0" encoding="UTF-8"?> | ||
| <beans xmlns="https://jakarta.ee/xml/ns/jakartaee" | ||
| xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" | ||
| xsi:schemaLocation="https://jakarta.ee/xml/ns/jakartaee https://jakarta.ee/xml/ns/jakartaee/beans_4_1.xsd"> | ||
|
|
||
| </beans> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| <configuration> | ||
| <appender name="STDOUT" class="ch.qos.logback.core.ConsoleAppender"> | ||
| <encoder> | ||
| <pattern>%d{HH:mm:ss.SSS} [%thread] %-5level %logger{36} - %msg%n</pattern> | ||
| </encoder> | ||
| </appender> | ||
|
|
||
| <appender name="FILE" class="ch.qos.logback.core.FileAppender"> | ||
| <file>logs/zombie-quest.log</file> | ||
| <append>true</append> | ||
| <encoder> | ||
| <pattern>%d{yyyy-MM-dd HH:mm:ss} [%thread] %-5level %logger{36} - %msg%n</pattern> | ||
| </encoder> | ||
| </appender> | ||
|
|
||
| <root level="debug"> | ||
| <appender-ref ref="STDOUT" /> | ||
| <appender-ref ref="FILE" /> | ||
| </root> | ||
| </configuration> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Поля wins и losses не инициализированы явно. Хотя int по умолчанию 0, явное присвоение значений в конструкторе повышает читаемость кода. [INFO]