Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,13 @@
</dependencyManagement>

<dependencies>
<!-- Source: https://mvnrepository.com/artifact/org.apache.commons/commons-lang3 -->
<dependency>
<groupId>org.apache.commons</groupId>
<artifactId>commons-lang3</artifactId>
<version>3.20.0</version>
<scope>compile</scope>
</dependency>
<dependency>
<groupId>jakarta.servlet</groupId>
<artifactId>jakarta.servlet-api</artifactId>
Expand Down Expand Up @@ -65,6 +72,17 @@
<artifactId>junit-jupiter-engine</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.junit.jupiter</groupId>
<artifactId>junit-jupiter-params</artifactId>
<scope>test</scope>
</dependency>
<dependency>
<groupId>org.mockito</groupId>
<artifactId>mockito-junit-jupiter</artifactId>
<version>5.21.0</version>
<scope>test</scope>
</dependency>

</dependencies>

Expand Down
18 changes: 18 additions & 0 deletions src/main/java/bakhtin/Action.java
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);

Copy link
Copy Markdown
Owner

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]

}
}
39 changes: 39 additions & 0 deletions src/main/java/bakhtin/ErrorHandlingFilter.java
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.*;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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);
}
}
}
22 changes: 22 additions & 0 deletions src/main/java/bakhtin/FrontController.java
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");

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Магическая строка '/hello' используется для перенаправления. Рекомендуется вынести пути в SessionConstants или использовать маппинг сервлетов. [WARNING]

}
}
208 changes: 208 additions & 0 deletions src/main/java/bakhtin/Quest.java
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;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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)) {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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<>();

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Статическое поле idGenerator в нестатическом внутреннем классе Question приведет к сквозной нумерации ID между всеми экземплярами квестов. Рекомендуется пересмотреть логику генерации ID для обеспечения изоляции данных. [WARNING]

String question;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The 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);
}

}

}

}
}
Loading