-
Notifications
You must be signed in to change notification settings - Fork 31
Bakhtin Denis Project Quest #20
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,18 @@ | ||
| package bakhtin; | ||
|
|
||
| import bakhtin.exceptions.IllegalActionException; | ||
|
|
||
| public enum Action { | ||
| ANSWER, RESTART, EXIT; | ||
|
|
||
| public static Action parse(String actionStr) { | ||
| if (actionStr != null) { | ||
| try { | ||
| return Action.valueOf(actionStr.toUpperCase()); | ||
| } catch (IllegalArgumentException e) { | ||
| throw new IllegalActionException("Action not found for: " + actionStr); | ||
| } | ||
| } | ||
| throw new IllegalActionException("Action not found for: " + actionStr); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,39 @@ | ||
| package bakhtin; | ||
|
|
||
| import bakhtin.exceptions.IllegalActionException; | ||
| import bakhtin.exceptions.NoActiveQuestException; | ||
| import bakhtin.exceptions.NoAnswerGivenException; | ||
| import bakhtin.exceptions.NoQuestionException; | ||
| import jakarta.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. Использование импорта через '' (jakarta.servlet.) считается плохой практикой. Следует импортировать только необходимые классы. [INFO] |
||
| import jakarta.servlet.annotation.WebFilter; | ||
| import java.io.IOException; | ||
|
|
||
| import static bakhtin.SessionConstants.*; | ||
|
|
||
| @WebFilter("/*") | ||
| public class ErrorHandlingFilter implements Filter { | ||
|
|
||
| @Override | ||
| public void doFilter(ServletRequest req, ServletResponse resp, FilterChain chain) | ||
| throws IOException, ServletException { | ||
| try { | ||
| chain.doFilter(req, resp); | ||
| } catch (NoActiveQuestException e) { | ||
| // TODO(log): log.error("No active quest exception", 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. Присутствует TODO вместо полноценного логирования. Вместо System.out или пустых блоков следует использовать SLF4J/Logback. [WARNING] |
||
| req.setAttribute(ERROR_MESSAGE_ATTR, "Квест не найден. Начните квест заново."); | ||
| req.getRequestDispatcher("/WEB-INF/hello.jsp").forward(req, resp); | ||
| } catch (NoAnswerGivenException e) { | ||
| // TODO(log): log.error("No answer given exception", e); | ||
| req.setAttribute(ERROR_MESSAGE_ATTR, "Не выбран ответ. Попробуйте ещё раз."); | ||
| req.getRequestDispatcher("/WEB-INF/hello.jsp").forward(req, resp); | ||
| } catch (NoQuestionException e) { | ||
| // TODO(log): log.error("No question exception", e); | ||
| req.setAttribute(ERROR_MESSAGE_ATTR, "Вопрос не найден. Начните квест заново."); | ||
| req.getRequestDispatcher("/WEB-INF/hello.jsp").forward(req, resp); | ||
| } catch (IllegalActionException e) { | ||
| // TODO(log): log.error("Illegal action exception", e); | ||
| req.setAttribute(ERROR_MESSAGE_ATTR, "Неверное действие. Попробуйте ещё раз"); | ||
| req.getRequestDispatcher("/WEB-INF/hello.jsp").forward(req, resp); | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package bakhtin; | ||
|
|
||
| import bakhtin.Quest.Question.Answer; | ||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.annotation.WebServlet; | ||
| import jakarta.servlet.http.HttpServlet; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpServletResponse; | ||
| import java.io.IOException; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| @WebServlet({"/bakhtin", "/bakhtin/", "/bakhtin/home", ""}) | ||
| public class FrontController extends HttpServlet { | ||
|
|
||
| @Override | ||
| protected void doGet(HttpServletRequest req, HttpServletResponse resp) | ||
| throws ServletException, IOException { | ||
| // Перенаправляем на quest сервлет | ||
| resp.sendRedirect(req.getContextPath() + "/hello"); | ||
|
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. Магическая строка '/hello' используется для перенаправления. Рекомендуется вынести пути в SessionConstants или использовать маппинг сервлетов. [WARNING] |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,208 @@ | ||
| package bakhtin; | ||
|
|
||
| import bakhtin.Quest.Question.Answer; | ||
| import java.io.Serializable; | ||
| import java.util.ArrayList; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
| import lombok.Getter; | ||
| import lombok.Setter; | ||
| import java.util.List; | ||
|
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.util.List. Чистота кода требует удаления лишних зависимостей. [INFO] |
||
|
|
||
| @Getter | ||
| public class Quest implements Serializable { | ||
|
|
||
| @Setter | ||
| Question startQuestion; | ||
| String name; | ||
| ConcurrentHashMap<Long, Question> questions; | ||
|
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 в классе, помеченном как Serializable, может быть избыточным, если не предполагается многопоточный доступ к состоянию квеста в рамках одной сессии. [INFO] |
||
|
|
||
| public Quest(String name) { | ||
| this.name = name; | ||
| questions = new ConcurrentHashMap<>(); | ||
| } | ||
|
|
||
| public static QuestBuilder builder() { | ||
| return new QuestBuilder(); | ||
| } | ||
|
|
||
|
|
||
| public Question addNewQuestion(String question, List<Answer> answers) { | ||
| Question q = new Question(question, answers); | ||
| if (q.getId().equals(1L)) { | ||
|
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. Магическое число '1L' для определения стартового вопроса. Лучше использовать явный флаг или передавать стартовый вопрос в конструктор Quest. [WARNING] |
||
| startQuestion = q; | ||
| } | ||
| questions.put(q.getId(), q); | ||
| return q; | ||
| } | ||
|
|
||
| public Question getQuestion(Long questionId) { | ||
| return questions.get(questionId); | ||
| } | ||
|
|
||
| public static class QuestBuilder { | ||
|
|
||
| private String name; | ||
| private Question startQuestion; | ||
| private final Map<Long, Question> questions = new 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. Поле 'questions' в QuestBuilder помечено как final, что ограничивает возможность сброса состояния билдера при повторном использовании. [INFO] |
||
|
|
||
| private QuestBuilder() { | ||
| } | ||
|
|
||
| public QuestBuilder name(String name) { | ||
| this.name = name; | ||
| return this; | ||
| } | ||
|
|
||
| public QuestBuilder question(Question question) { | ||
| if (question.getId() == 1L) { | ||
| startQuestion = question; | ||
| } | ||
| questions.put(question.getId(), question); | ||
| return this; | ||
| } | ||
|
|
||
| public QuestBuilder startQuestion(Question startQuestion) { | ||
| this.startQuestion = startQuestion; | ||
| questions.put(startQuestion.getId(), startQuestion); | ||
| return this; | ||
| } | ||
|
|
||
| public Quest build() { | ||
| Quest quest = new Quest(name); | ||
| quest.setStartQuestion(startQuestion); | ||
| quest.getQuestions().putAll(questions); | ||
| return quest; | ||
| } | ||
|
|
||
| } | ||
|
|
||
|
|
||
| @Getter | ||
| public static class Question implements Serializable { | ||
|
|
||
| private static final long serialVersionUID = 1L; | ||
| private boolean finish; | ||
| private boolean win; | ||
| private static final AtomicLong idGenerator = new AtomicLong(0); | ||
|
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. Статическое поле idGenerator в нестатическом внутреннем классе Question приведет к сквозной нумерации ID между всеми экземплярами квестов. Рекомендуется пересмотреть логику генерации ID для обеспечения изоляции данных. [WARNING] |
||
| String question; | ||
|
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. Поле 'question' имеет уровень доступа по умолчанию (package-private). Согласно принципам инкапсуляции, следует использовать модификатор private. [WARNING] |
||
| ConcurrentHashMap<Long, Answer> answers = new ConcurrentHashMap<>(); | ||
| Long id; | ||
|
|
||
| public static QuestionBuilder builder() { | ||
| return new QuestionBuilder(); | ||
| } | ||
|
|
||
| public Question(boolean win, String question) { | ||
| this.win = win; | ||
| this.question = question; | ||
| finish = true; | ||
| id = idGenerator.incrementAndGet(); | ||
| } | ||
|
|
||
| public Question(String question, List<Answer> answers) { | ||
| this.question = question; | ||
| id = idGenerator.incrementAndGet(); | ||
| for (Answer answer : answers) { | ||
| this.answers.put(answer.id, answer); | ||
|
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. Прямое обращение к полю 'answer.id' нарушает инкапсуляцию, даже если это внутренний класс. Следует использовать геттер. [WARNING] |
||
| } | ||
| finish = false; | ||
| } | ||
|
|
||
| public Answer getAnswer(Long answerId) { | ||
| return answers.get(answerId); | ||
| } | ||
|
|
||
| public static class QuestionBuilder { | ||
|
|
||
| private String question; | ||
| private List<Answer> answers = new ArrayList<>(); | ||
| private boolean terminal; | ||
| private boolean win; | ||
|
|
||
| private QuestionBuilder() { | ||
| } | ||
|
|
||
| public QuestionBuilder question(String question) { | ||
| this.question = question; | ||
| return this; | ||
| } | ||
|
|
||
| public QuestionBuilder answer(Answer answer) { | ||
| answers.add(answer); | ||
| return this; | ||
| } | ||
|
|
||
| public QuestionBuilder win(boolean win) { | ||
| terminal = true; | ||
| this.win = win; | ||
| return this; | ||
| } | ||
|
|
||
| public Question build() { | ||
| if (terminal) { | ||
| return new Question(win, question); | ||
| } else { | ||
| return new Question(question, answers); | ||
| } | ||
| } | ||
|
|
||
| } | ||
|
|
||
| @Getter | ||
| public static class Answer implements Serializable { | ||
|
|
||
| private static final long serialVersionUID = 1L; | ||
|
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. serialVersionUID установлен вручную в 1L. Это хорошая практика для Serializable, но стоит убедиться, что все вложенные классы также следуют этому правилу консистентно. [INFO] |
||
| private static final AtomicLong idGenerator = new AtomicLong(0); | ||
|
|
||
| private final Long id; | ||
| private final String text; | ||
|
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. Поле 'text' в классе Answer объявлено как final, но инициализируется через генератор ID, что делает класс менее гибким для тестирования без создания побочных эффектов. [INFO] |
||
| private @Setter | ||
| Question nextQuestion; | ||
|
|
||
|
|
||
| public static AnswerBuilder builder() { | ||
| return new AnswerBuilder(); | ||
| } | ||
|
|
||
| public Answer(String text) { | ||
| this.text = text; | ||
| this.nextQuestion = null; | ||
| id = idGenerator.incrementAndGet(); | ||
| } | ||
|
|
||
| public Answer(String text, Question nextQuestion) { | ||
| this.text = text; | ||
| this.nextQuestion = nextQuestion; | ||
| id = idGenerator.incrementAndGet(); | ||
| } | ||
|
|
||
| public static class AnswerBuilder { | ||
|
|
||
| private String answer; | ||
| private Question nextQuestion; | ||
|
|
||
| public AnswerBuilder answer(String answer) { | ||
| this.answer = answer; | ||
| return this; | ||
| } | ||
|
|
||
| public AnswerBuilder nextQuestion(Question nextQuestion) { | ||
| this.nextQuestion = nextQuestion; | ||
| return this; | ||
| } | ||
|
|
||
| private AnswerBuilder() { | ||
| } | ||
|
|
||
| public Answer build() { | ||
| return new Answer(answer, nextQuestion); | ||
| } | ||
|
|
||
| } | ||
|
|
||
| } | ||
|
|
||
| } | ||
| } | ||
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.
Дублирование логики выброса исключения. Условие if (actionStr != null) можно упростить, используя Optional или более лаконичную проверку. [INFO]