-
Notifications
You must be signed in to change notification settings - Fork 30
Project 3 Final Pantera Bekk I.V. Detached #21
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
0f17cd6
3c042bb
be7167d
4a24901
293a509
67514e7
1d0723f
d1f6695
e1f8d69
7728861
b02e8a6
29ff730
dc8f10d
cc2697a
d86d99e
ad0e8cc
0668a1c
8373be8
ce0357e
e358fea
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,4 @@ | ||
| package com.javarush.bekk.cmd; | ||
|
|
||
| public class EndPage implements Command { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,25 @@ | ||
| package com.javarush.bekk.cmd; | ||
|
|
||
| import com.javarush.bekk.entity.Answer; | ||
| import com.javarush.bekk.entity.Question; | ||
| import com.javarush.bekk.repository.QuestionRepository; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpSession; | ||
| import lombok.AllArgsConstructor; | ||
|
|
||
| import java.util.Collection; | ||
|
|
||
| @SuppressWarnings("unused") | ||
| @AllArgsConstructor | ||
| public class StartPage implements Command { | ||
|
|
||
| private final QuestionRepository questionRepository; | ||
|
|
||
| @Override | ||
| public String doGet(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| Question question = questionRepository.get(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) ограничивает гибкость. Начальный вопрос должен определяться конфигурацией или параметром, чтобы логику не приходилось менять при обновлении данных. |
||
| session.setAttribute("question", question); | ||
| return getView(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| package com.javarush.bekk.cmd; | ||
|
|
||
| import com.javarush.bekk.entity.Answer; | ||
| import com.javarush.bekk.entity.Question; | ||
| import com.javarush.bekk.repository.QuestionRepository; | ||
| import com.javarush.bekk.util.Constant; | ||
| import jakarta.servlet.http.HttpServletRequest; | ||
| import jakarta.servlet.http.HttpSession; | ||
| import lombok.AllArgsConstructor; | ||
|
|
||
| import java.util.Collection; | ||
|
|
||
| @AllArgsConstructor | ||
| public class TestPage implements Command { | ||
|
|
||
| private final QuestionRepository questionRepository; | ||
|
|
||
|
|
||
| @Override | ||
| public String doPost(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| String[] answers = (request.getParameter("answer")).split(","); | ||
|
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 для параметра 'answer'. Если пользователь перейдет по ссылке напрямую, возникнет NullPointerException.
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. Разделение логики через запятую в параметре запроса — хрупкое решение. Лучше передавать ID ответа и ID следующего вопроса как отдельные параметры. |
||
| int answer = Integer.parseInt(answers[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. Прямой парсинг Integer.parseInt без обработки NumberFormatException приведет к падению сервера при некорректном вводе. |
||
| int nextQuestionId = Integer.parseInt(answers[1]); | ||
| if (answer == Constant.RIGHT_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 или Question (инкапсуляция). |
||
| Question question = questionRepository.get(nextQuestionId); | ||
| session.setAttribute("question", question); | ||
| return getView(); | ||
| } else if (answer == Constant.LAST_RIGHT_ANSWER){ | ||
| return "win-page"; | ||
| } else { | ||
| return "end-page"; | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| package com.javarush.bekk.cmd; | ||
|
|
||
| public class WinPage implements Command { | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,67 @@ | ||
| package com.javarush.bekk.config; | ||
|
|
||
| import com.javarush.bekk.entity.Answer; | ||
| import com.javarush.bekk.entity.Question; | ||
| import com.javarush.bekk.repository.QuestionRepository; | ||
| import lombok.AllArgsConstructor; | ||
|
|
||
| @AllArgsConstructor | ||
| public class Config { | ||
|
|
||
| private final QuestionRepository questionRepository; | ||
|
|
||
| public void fillRepository() { | ||
| Question question1 = buildQuestion(1L, "S — Single Responsibility Principle (Принцип единственной ответственности)"); | ||
|
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. Метод fillRepository перегружен логикой создания объектов. Рекомендуется использовать паттерн Builder или вынести создание данных в отдельный сервис/JSON-файл. |
||
| question1.getAnswers().add(new Answer(5L, question1.getId(), "Каждый класс должен иметь только одну причину для изменения, " + | ||
| "то есть выполнять только одну задачу или отвечать за одну ответственность. Это повышает сплочённость (cohesion) и упрощает поддержку кода", | ||
| question1.getId() + 1)); | ||
| question1.getAnswers().add(new Answer(2L, question1.getId(), "Каждый класс должен иметь много причин для изменения, " + | ||
| "то есть выполнять много задач или отвечать за несколько ответственностей. Это повышает сплочённость (cohesion) и упрощает поддержку кода", | ||
| question1.getId() + 1)); | ||
| questionRepository.create(question1); | ||
|
|
||
| Question question2 = buildQuestion(2L, "O — Open/Closed Principle (Принцип открытости/закрытости)"); | ||
| question2.getAnswers().add(new Answer(5L, question2.getId(), "Классы должны быть открыты для расширения (добавления нового функционала), " + | ||
| "но закрыты для модификации (изменения существующего кода). Это достигается через абстракции (интерфейсы, абстрактные классы) и полиморфизм", | ||
| question2.getId() + 1)); | ||
| question2.getAnswers().add(new Answer(2L, question2.getId(), "Классы должны быть закрыты для расширения (добавления нового функционала), " + | ||
| "но открыты для модификации (изменения существующего кода)", | ||
| question2.getId() + 1)); | ||
| questionRepository.create(question2); | ||
|
|
||
| Question question3 = buildQuestion(3L, "Liskov Substitution Principle (Принцип подстановки Барбары Лисков)"); | ||
| question3.getAnswers().add(new Answer(5L, question3.getId(), "Объекты базового класса должны быть заменяемы объектами " + | ||
| "производных классов без изменения поведения программы. Это гарантирует, что подклассы не нарушают контракты, заданные базовым классом", | ||
| question3.getId() + 1)); | ||
| question3.getAnswers().add(new Answer(2L, question3.getId(), "Объекты базового класса не должны быть заменяемы объектами производных классов " + | ||
| "без изменения поведения программы. Это гарантирует, что подклассы не нарушают контракты, заданные базовым классом", | ||
| question3.getId() + 1)); | ||
| questionRepository.create(question3); | ||
|
|
||
| Question question4 = buildQuestion(4L, "Interface Segregation Principle (Принцип разделения интерфейсов)"); | ||
| question4.getAnswers().add(new Answer(5L, question4.getId(), "Клиенты не должны быть вынуждены реализовывать интерфейсы, которые они не используют. " + | ||
| "Интерфейсы должны быть узкоспециализированными, чтобы классы реализовывали только необходимый функционал", | ||
| question4.getId() + 1)); | ||
| question4.getAnswers().add(new Answer(2L, question4.getId(), "Клиенты должны реализовывать интерфейсы, которые они не используют. " + | ||
| "Интерфейсы должны быть универсальными, чтобы классы реализовывали универсальный функционал", | ||
| question4.getId() + 1)); | ||
| questionRepository.create(question4); | ||
|
|
||
| Question question5 = buildQuestion(5L, "D — Dependency Inversion Principle (Принцип инверсии зависимостей)"); | ||
| question5.getAnswers().add(new Answer(100L, question5.getId(), "Модули высокого уровня не должны зависеть от модулей низкого уровня; " + | ||
| "оба должны зависеть от абстракций. Абстракции не должны зависеть от деталей реализации, а детали — от абстракций. Это снижает связанность (coupling)", | ||
| question5.getId() + 1)); | ||
| question5.getAnswers().add(new Answer(2L, question5.getId(), "Модули высокого уровня должны зависеть от модулей низкого уровня. " + | ||
| "Это повышает связанность и упрощает написание программы", | ||
| question5.getId() + 1)); | ||
| questionRepository.create(question5); | ||
| } | ||
|
|
||
| private static Question buildQuestion(Long questionId, String text) { | ||
| return Question.builder() | ||
| .id(questionId) | ||
| .text(text) | ||
| .build(); | ||
| } | ||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| package com.javarush.khmelov.config; | ||
| package com.javarush.bekk.config; | ||
|
|
||
| import lombok.SneakyThrows; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,10 @@ | ||
| package com.javarush.khmelov.controller; | ||
| package com.javarush.bekk.controller; | ||
|
|
||
| import com.javarush.khmelov.cmd.Command; | ||
| import com.javarush.khmelov.config.Winter; | ||
| import com.javarush.khmelov.entity.Role; | ||
| import com.javarush.bekk.cmd.Command; | ||
| import com.javarush.bekk.config.Config; | ||
| import com.javarush.bekk.config.Winter; | ||
| import jakarta.servlet.ServletConfig; | ||
| import jakarta.servlet.ServletContext; | ||
| import jakarta.servlet.ServletException; | ||
| import jakarta.servlet.annotation.WebServlet; | ||
| import jakarta.servlet.http.HttpServlet; | ||
|
|
@@ -12,7 +13,7 @@ | |
|
|
||
| import java.io.IOException; | ||
|
|
||
| @WebServlet({"", "/home", "/list-user", "/edit-user"}) | ||
|
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. Инициализация поля через статический метод Winter.find затрудняет юнит-тестирование контроллера. Стоит использовать внедрение зависимостей. |
||
| @WebServlet({"", "/test-page", "/end-page", "/win-page", "/start-page"}) | ||
| public class FrontController extends HttpServlet { | ||
|
|
||
| private final HttpResolver httpResolver = Winter.find(HttpResolver.class); | ||
|
|
@@ -27,7 +28,11 @@ protected void doGet(HttpServletRequest req, HttpServletResponse resp) throws Se | |
|
|
||
| @Override | ||
| public void init(ServletConfig config) { | ||
| config.getServletContext().setAttribute("roles", Role.values()); | ||
| Config config1 = Winter.find(Config.class); | ||
| config1.fillRepository(); | ||
|
|
||
| ServletContext servletContext = config.getServletContext(); | ||
| servletContext.setAttribute(Config.class.getName(), config1); | ||
| } | ||
|
|
||
| private static String getJsp(String view) { | ||
|
|
@@ -40,4 +45,9 @@ protected void doPost(HttpServletRequest req, HttpServletResponse resp) throws S | |
| String redirect = command.doPost(req); | ||
| resp.sendRedirect(redirect); | ||
| } | ||
|
|
||
| /*@Override | ||
| public void init(ServletConfig config) { | ||
| config.getServletContext().setAttribute("roles", Role.values()); | ||
| }*/ | ||
| } | ||
|
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. Использование redirect для всех POST-запросов — хорошая практика (PRG), но здесь не передаются сообщения об ошибках или состояние. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package com.javarush.bekk.entity; | ||
|
|
||
| public interface AbstractEntity { | ||
|
|
||
| Long getId(); | ||
|
|
||
| void setId(Long id); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package com.javarush.bekk.entity; | ||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| @Data | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @Builder | ||
| public class Answer implements AbstractEntity { | ||
|
|
||
| private Long id; | ||
|
|
||
| private Long questionId; | ||
|
|
||
| private String text; | ||
|
|
||
| private Long nextQuestionId; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| package com.javarush.bekk.entity; | ||
|
|
||
| public enum GameState { | ||
| WIN, LOSER, PLAY | ||
| } | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| package com.javarush.bekk.entity; | ||
|
|
||
| import lombok.AllArgsConstructor; | ||
| import lombok.Builder; | ||
| import lombok.Data; | ||
| import lombok.NoArgsConstructor; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
|
|
||
| @Data | ||
| @NoArgsConstructor | ||
| @AllArgsConstructor | ||
| @Builder | ||
| public class Question implements AbstractEntity{ /*1 шаг в игре*/ | ||
|
|
||
| private Long id; //пользователь | ||
|
|
||
| //private Long questId; | ||
|
|
||
| private String text; | ||
|
|
||
| //private GameState gameState; | ||
|
|
||
| private final Collection<Answer> answers = new ArrayList<>(); | ||
|
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. Коллекция 'answers' инициализируется сразу. В сочетании с @DaTa от Lombok это может привести к неожиданному поведению при десериализации или копировании. |
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.javarush.bekk.repository; | ||
|
|
||
| import com.javarush.bekk.entity.Answer; | ||
|
|
||
| import java.util.ArrayList; | ||
| import java.util.Collection; | ||
| import java.util.stream.Stream; | ||
|
|
||
| public class AnswerRepository extends BaseRepository<Answer> { | ||
|
|
||
|
|
||
|
|
||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| package com.javarush.bekk.repository; | ||
|
|
||
| import com.javarush.bekk.entity.AbstractEntity; | ||
|
|
||
| import java.util.Collection; | ||
| import java.util.Map; | ||
| import java.util.concurrent.ConcurrentHashMap; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
|
|
||
| public abstract class BaseRepository<T extends AbstractEntity> implements Repository<T> { | ||
| /*<T extends AbstractEntity> надо гарантировать (есть setId,getId), что Long в мапе есть абсолютно у любой сущности | ||
| поэтому в репозитории ключом будет Long всегда*/ | ||
|
|
||
| protected final Map<Long, T> map = 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. Поле 'map' объявлено как protected. Согласно принципам инкапсуляции, лучше сделать его private и предоставлять доступ через методы. |
||
|
|
||
| public final AtomicLong id = new AtomicLong(0L); | ||
|
|
||
| @Override | ||
| public Collection<T> getAll() { | ||
| return map.values(); | ||
| } | ||
|
|
||
| @Override | ||
| public T get(long id) { | ||
| return map.get(id); | ||
| } | ||
|
|
||
| @Override | ||
| public void create(T entity) { | ||
| entity.setId(id.incrementAndGet()); | ||
| update(entity); | ||
|
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. Метод create вызывает update, что избыточно. Логику вставки и обновления стоит четко разделить для чистоты кода. |
||
| } | ||
|
|
||
| @Override | ||
| public void update(T entity) { | ||
| map.put(entity.getId(), entity); | ||
| } | ||
|
|
||
| @Override | ||
| public void delete(T entity) { | ||
| map.remove(entity.getId()); | ||
| } | ||
|
|
||
| /*protected boolean nullOrEquals(Object patternField, Object repoField) { | ||
| return patternField == null || patternField.equals(repoField); | ||
| }*/ | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| package com.javarush.bekk.repository; | ||
|
|
||
| import com.javarush.bekk.entity.Answer; | ||
| import com.javarush.bekk.entity.Question; | ||
|
|
||
| import java.util.HashMap; | ||
| import java.util.Map; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
|
|
||
| public class QuestionRepository extends BaseRepository<Question> { | ||
|
|
||
| private final Map<Long, Question> map = new HashMap<>(); | ||
|
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. Поле 'map' дублирует функционал базового класса BaseRepository. Это приводит к путанице и лишнему потреблению памяти. |
||
|
|
||
| public static final AtomicLong id = new AtomicLong(System.currentTimeMillis()); | ||
|
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. Поле 'id' является статическим, но при этом перекрывает аналогичное поле в базовом классе. Это нарушает принципы ООП и затрудняет поддержку. |
||
|
|
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| package com.javarush.bekk.repository; | ||
|
|
||
| import java.util.Collection; | ||
|
|
||
| public interface Repository<T> { | ||
|
|
||
| Collection<T> getAll(); | ||
|
|
||
| //Stream<T> find(T pattern); | ||
|
|
||
| T get(long id); | ||
|
|
||
| void create(T entity); | ||
|
|
||
| void update(T entity); | ||
|
|
||
| void delete(T entity); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.javarush.bekk.service; | ||
|
|
||
| import com.javarush.bekk.entity.Answer; | ||
| import com.javarush.bekk.repository.QuestionRepository; | ||
|
|
||
| public class AnswerService { | ||
| private final QuestionRepository questionRepository; | ||
|
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. В AnswerService внедряется QuestionRepository вместо AnswerRepository. Нарушена логика слоев. |
||
|
|
||
| public AnswerService(QuestionRepository questionRepository) { | ||
| this.questionRepository = questionRepository; | ||
| } | ||
|
|
||
| } | ||
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.
Аннотация @SuppressWarnings("unused") часто скрывает проблемы архитектуры. Если класс используется через рефлексию, лучше настроить саму IDE или использовать документацию, а не подавлять предупреждения компилятора.