-
Notifications
You must be signed in to change notification settings - Fork 27
Чеботарев - Project 3# Quest #9
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
df51f45
3ef636b
be636a0
22021a4
3e22677
51e6564
1ac2d69
6f2c0e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,23 +1,24 @@ | ||
| package com.javarush.khmelov.cmd; | ||
| package com.javarush.chebotarev.cmd; | ||
|
|
||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
|
|
||
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public interface Command { | ||
| public abstract class Command { | ||
|
|
||
| default String doGet(HttpServletRequest request) { | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { | ||
| return getView(); | ||
| } | ||
|
|
||
| default String doPost(HttpServletRequest request) { | ||
| public String doPost(HttpServletRequest req) { | ||
| return getView(); | ||
| } | ||
|
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. Логика преобразования CamelCase в kebab-style может быть упрощена с использованием регулярных выражений Java 21. [INFO] |
||
|
|
||
| default String getView() { | ||
| protected String getView() { | ||
| String simpleName = this.getClass().getSimpleName(); | ||
| return convertCamelCaseToKebabStyle(simpleName); | ||
| return "/" + convertCamelCaseToKebabStyle(simpleName); | ||
| } | ||
|
|
||
| private static String convertCamelCaseToKebabStyle(String string) { | ||
|
|
@@ -32,6 +33,4 @@ private static String convertCamelCaseToKebabStyle(String string) { | |
| ? snakeName.substring(1) | ||
|
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] |
||
| : snakeName; | ||
| } | ||
|
|
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package com.javarush.chebotarev.cmd; | ||
|
|
||
| import com.javarush.chebotarev.component.Attribute; | ||
| import com.javarush.chebotarev.component.Go; | ||
| import com.javarush.chebotarev.component.Utils; | ||
| import com.javarush.chebotarev.quest.CurrentQuest; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpSession; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| public class ContinueQuest extends Command { | ||
|
|
||
| @Override | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { | ||
| HttpSession currentSession = req.getSession(); | ||
| CurrentQuest currentQuest = Utils.extractAttribute( | ||
| currentSession, | ||
| Attribute.CURRENT_QUEST, | ||
| CurrentQuest.class | ||
| ); | ||
| String view; | ||
| if (!currentQuest.isStarted()) { | ||
| view = Go.NEW_QUEST; | ||
| } else if (!currentQuest.isDone()) { | ||
| view = Go.QUEST; | ||
| } else { | ||
| view = Go.RESULT; | ||
| } | ||
| return view; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| package com.javarush.chebotarev.cmd; | ||
|
|
||
| import com.fasterxml.jackson.databind.ObjectMapper; | ||
| import com.javarush.chebotarev.component.Go; | ||
| import com.javarush.chebotarev.component.ObjectRepository; | ||
| import com.javarush.chebotarev.component.QuestService; | ||
| import com.javarush.chebotarev.quest.Quest; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| public class Editor extends Command { | ||
|
|
||
| @Override | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { | ||
| return Go.EDITOR; | ||
| } | ||
|
|
||
| @Override | ||
| public String doPost(HttpServletRequest req) { | ||
| try { | ||
| req.setCharacterEncoding("UTF-8"); | ||
|
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. Кодировка UTF-8 жестко захардкожена. Рекомендуется использовать StandardCharsets.UTF_8.name(). [INFO] |
||
| ObjectMapper mapper = ObjectRepository.find(ObjectMapper.class); | ||
| Quest quest = mapper.readValue(req.getReader(), Quest.class); | ||
|
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. Чтение всего тела запроса через getReader() и маппинг в объект может упасть с ошибкой памяти при очень больших файлах квестов. [WARNING] |
||
| QuestService questService = ObjectRepository.find(QuestService.class); | ||
| questService.saveQuest(quest); | ||
|
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. Результат сохранения квеста игнорируется. Если saveQuest возвращает путь к файлу, возможно, его стоит использовать для логирования или информирования пользователя. [INFO] |
||
| } catch (IOException e) { | ||
| throw new RuntimeException(e); | ||
|
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. Оборачивание IOException в RuntimeException без контекста затрудняет отладку. Рекомендуется использовать пользовательские исключения или информативные сообщения. [WARNING] |
||
| } | ||
| return Go.ROOT; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package com.javarush.chebotarev.cmd; | ||
|
|
||
| import com.javarush.chebotarev.component.*; | ||
| import com.javarush.chebotarev.quest.QuestMetadata; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpSession; | ||
|
|
||
| import java.util.List; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| public class MainMenu extends Command { | ||
|
|
||
| @Override | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { | ||
|
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. Метод не содержит логирования. Для Senior уровня важно отслеживать действия пользователя (например, вход в главное меню). [INFO] |
||
| HttpSession currentSession = req.getSession(); | ||
| QuestService questService = ObjectRepository.find(QuestService.class); | ||
|
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. Поиск сервиса через ObjectRepository в каждом вызове метода doGet создает избыточную нагрузку. Рекомендуется использовать Dependency Injection. [WARNING] |
||
| List<QuestMetadata> availableQuests | ||
| = questService.obtainAvailableQuests(servlet.getServletContext()); | ||
|
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. Метод obtainAvailableQuests может быть тяжелым (чтение с диска). Его стоит кэшировать или выполнять асинхронно. [WARNING] |
||
| currentSession.setAttribute(Attribute.AVAILABLE_QUESTS, availableQuests); | ||
| Statistics statistics = Utils.tryExtractAttribute( | ||
| currentSession, | ||
| Attribute.STATISTICS, | ||
| Statistics.class | ||
| ); | ||
| if (statistics == null) { | ||
| statistics = new Statistics(); | ||
| currentSession.setAttribute(Attribute.STATISTICS, statistics); | ||
|
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. Инициализация нового объекта статистики в контроллере нарушает принцип единственной ответственности (Single Responsibility). [WARNING] |
||
| } | ||
| return getView(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package com.javarush.chebotarev.cmd; | ||
|
|
||
| import com.javarush.chebotarev.component.*; | ||
| import com.javarush.chebotarev.quest.CurrentQuest; | ||
| import com.javarush.chebotarev.quest.Quest; | ||
| import com.javarush.chebotarev.quest.QuestMetadata; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpSession; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| public class NewQuest extends Command { | ||
|
|
||
| @Override | ||
| @SuppressWarnings("unchecked") | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { | ||
| HttpSession currentSession = req.getSession(); | ||
| int selectedQuestIndex = getSelectedQuestIndex(req); | ||
| List<QuestMetadata> availableQuests = Utils.extractAttribute( | ||
|
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. Список доступных квестов извлекается из сессии. Если сессия истекла или атрибут не установлен, возникнет ошибка при получении по индексу. [ERROR] |
||
| currentSession, | ||
| Attribute.AVAILABLE_QUESTS, | ||
| ArrayList.class | ||
| ); | ||
| QuestService questService = ObjectRepository.find(QuestService.class); | ||
| QuestMetadata selectedQuest = availableQuests.get(selectedQuestIndex); | ||
| Quest quest = questService.loadQuest( | ||
| selectedQuest, | ||
| servlet.getServletContext() | ||
| ); | ||
| CurrentQuest currentQuest = new CurrentQuest(quest); | ||
| currentSession.setAttribute(Attribute.CURRENT_QUEST, currentQuest); | ||
|
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. Объект currentQuest сохраняется в сессию под ключом CURRENT_QUEST, перезаписывая старый прогресс без предупреждения пользователя. [WARNING] |
||
| return getView(); | ||
| } | ||
|
|
||
| private int getSelectedQuestIndex(HttpServletRequest req) { | ||
| String questIndexString = req.getParameter(Parameter.QUEST_INDEX); | ||
| return Integer.parseInt(questIndexString); | ||
|
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. Метод Integer.parseInt может выбросить NumberFormatException, если параметр отсутствует или не является числом. Следует добавить валидацию входных данных. [ERROR] |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package com.javarush.chebotarev.cmd; | ||
|
|
||
| import com.javarush.chebotarev.component.*; | ||
| import com.javarush.chebotarev.quest.CurrentQuest; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpSession; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| public class NextStage extends Command { | ||
|
|
||
| @Override | ||
| public String doGet(HttpServletRequest req, HttpServlet servlet) { | ||
| HttpSession currentSession = req.getSession(); | ||
| CurrentQuest currentQuest = Utils.extractAttribute( | ||
|
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. Переменная currentQuest извлекается из сессии, но не проверяется на null перед использованием. [ERROR] |
||
| currentSession, | ||
| Attribute.CURRENT_QUEST, | ||
| CurrentQuest.class | ||
| ); | ||
| int nextNodeId = getSelectedNextNodeId(req); | ||
| currentQuest.nextStage(nextNodeId); | ||
|
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. Вызов currentQuest.nextStage(nextNodeId) изменяет состояние объекта. В многопоточной среде (сервлеты) это может привести к race condition, если объект в сессии не синхронизирован. [ERROR] |
||
| String view; | ||
| if (currentQuest.isDone()) { | ||
|
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. Метод isDone() используется для определения перехода на страницу результатов. Логика завершения квеста должна быть инкапсулирована внутри CurrentQuest. [INFO] |
||
| view = Go.RESULT; | ||
| Statistics statistics = Utils.extractAttribute( | ||
| currentSession, | ||
| Attribute.STATISTICS, | ||
| Statistics.class | ||
| ); | ||
| if (currentQuest.isVictory()) { | ||
| statistics.incVictories(); | ||
|
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. Обновление статистики напрямую в сессионном объекте. Рекомендуется выделить логику обновления статистики в отдельный сервис. [WARNING] |
||
| } else { | ||
| statistics.incDefeats(); | ||
| } | ||
| } else { | ||
| view = Go.QUEST; | ||
| } | ||
| return view; | ||
| } | ||
|
|
||
| private int getSelectedNextNodeId(HttpServletRequest req) { | ||
| String nextNodeIdString = req.getParameter(Parameter.NEXT_NODE_ID); | ||
|
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. Имя параметра NEXT_NODE_ID лучше вынести в константу класса Parameter для исключения опечаток. [INFO] |
||
| return Integer.parseInt(nextNodeIdString); | ||
|
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 или формат является потенциальным источником RuntimeException. [ERROR] |
||
| } | ||
| } | ||
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.
Методы doGet и doPost возвращают строку с путем. Было бы более наглядно использовать перечисления (Enum) для навигации. [INFO]