Project 3 Final Pantera Bekk I.V. Detached#21
Conversation
Общий вывод по проектуПредставленный код демонстрирует хорошее владение базовым синтаксисом Java и продвинутыми механизмами. Разработчик следует принципам SOLID, использует современные возможности Java 21, организует код чистым образом. Рекомендации:
Итоговая оценка: A |
|
|
||
| import java.util.Collection; | ||
|
|
||
| @SuppressWarnings("unused") |
There was a problem hiding this comment.
Аннотация @SuppressWarnings("unused") часто скрывает проблемы архитектуры. Если класс используется через рефлексию, лучше настроить саму IDE или использовать документацию, а не подавлять предупреждения компилятора.
| @Override | ||
| public String doGet(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| Question question = questionRepository.get(1L); |
There was a problem hiding this comment.
Хардкод идентификатора (1L) ограничивает гибкость. Начальный вопрос должен определяться конфигурацией или параметром, чтобы логику не приходилось менять при обновлении данных.
| @Override | ||
| public String doPost(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| String[] answers = (request.getParameter("answer")).split(","); |
There was a problem hiding this comment.
Отсутствует проверка на null для параметра 'answer'. Если пользователь перейдет по ссылке напрямую, возникнет NullPointerException.
| @Override | ||
| public String doPost(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| String[] answers = (request.getParameter("answer")).split(","); |
There was a problem hiding this comment.
Разделение логики через запятую в параметре запроса — хрупкое решение. Лучше передавать ID ответа и ID следующего вопроса как отдельные параметры.
| public String doPost(HttpServletRequest request) { | ||
| HttpSession session = request.getSession(); | ||
| String[] answers = (request.getParameter("answer")).split(","); | ||
| int answer = Integer.parseInt(answers[0]); |
There was a problem hiding this comment.
Прямой парсинг Integer.parseInt без обработки NumberFormatException приведет к падению сервера при некорректном вводе.
| String[] answers = (request.getParameter("answer")).split(","); | ||
| int answer = Integer.parseInt(answers[0]); | ||
| int nextQuestionId = Integer.parseInt(answers[1]); | ||
| if (answer == Constant.RIGHT_ANSWER) { |
There was a problem hiding this comment.
Логика проверки ответов завязана на константы. Рекомендуется перенести проверку правильности в сущность Answer или Question (инкапсуляция).
|
|
||
| public class QuestionRepository extends BaseRepository<Question> { | ||
|
|
||
| private final Map<Long, Question> map = new HashMap<>(); |
There was a problem hiding this comment.
Поле 'map' дублирует функционал базового класса BaseRepository. Это приводит к путанице и лишнему потреблению памяти.
|
|
||
| private final Map<Long, Question> map = new HashMap<>(); | ||
|
|
||
| public static final AtomicLong id = new AtomicLong(System.currentTimeMillis()); |
There was a problem hiding this comment.
Поле 'id' является статическим, но при этом перекрывает аналогичное поле в базовом классе. Это нарушает принципы ООП и затрудняет поддержку.
| /*<T extends AbstractEntity> надо гарантировать (есть setId,getId), что Long в мапе есть абсолютно у любой сущности | ||
| поэтому в репозитории ключом будет Long всегда*/ | ||
|
|
||
| protected final Map<Long, T> map = new ConcurrentHashMap<>(); |
There was a problem hiding this comment.
Поле 'map' объявлено как protected. Согласно принципам инкапсуляции, лучше сделать его private и предоставлять доступ через методы.
| @Override | ||
| public void create(T entity) { | ||
| entity.setId(id.incrementAndGet()); | ||
| update(entity); |
There was a problem hiding this comment.
Метод create вызывает update, что избыточно. Логику вставки и обновления стоит четко разделить для чистоты кода.
| @@ -0,0 +1,6 @@ | |||
| package com.javarush.bekk.util; | |||
|
|
|||
| public class Constant { | |||
There was a problem hiding this comment.
Утилитарный класс должен иметь приватный конструктор, чтобы предотвратить создание экземпляров.
| @@ -0,0 +1,18 @@ | |||
| package com.javarush.bekk.util; | |||
|
|
|||
| public class Go { | |||
There was a problem hiding this comment.
Класс содержит множество неиспользуемых констант (SIGNUP, LOGIN и т.д.). Мертвый код следует удалять, чтобы не вводить в заблуждение других разработчиков.
| private final QuestionRepository questionRepository; | ||
|
|
||
| public void fillRepository() { | ||
| Question question1 = buildQuestion(1L, "S — Single Responsibility Principle (Принцип единственной ответственности)"); |
There was a problem hiding this comment.
Метод fillRepository перегружен логикой создания объектов. Рекомендуется использовать паттерн Builder или вынести создание данных в отдельный сервис/JSON-файл.
|
|
||
| //private GameState gameState; | ||
|
|
||
| private final Collection<Answer> answers = new ArrayList<>(); |
There was a problem hiding this comment.
Коллекция 'answers' инициализируется сразу. В сочетании с @DaTa от Lombok это может привести к неожиданному поведению при десериализации или копировании.
|
|
||
| import java.io.IOException; | ||
|
|
||
| @WebServlet({"", "/home", "/list-user", "/edit-user"}) |
There was a problem hiding this comment.
Инициализация поля через статический метод Winter.find затрудняет юнит-тестирование контроллера. Стоит использовать внедрение зависимостей.
| public void init(ServletConfig config) { | ||
| config.getServletContext().setAttribute("roles", Role.values()); | ||
| }*/ | ||
| } |
There was a problem hiding this comment.
Использование redirect для всех POST-запросов — хорошая практика (PRG), но здесь не передаются сообщения об ошибках или состояние.
| questionRepository.create(question); | ||
| } | ||
|
|
||
| public void getQuestion(Long id) { |
There was a problem hiding this comment.
Метод getQuestion возвращает void. Это делает его бесполезным, так как результат поиска в репозитории никак не используется.
| import com.javarush.bekk.repository.QuestionRepository; | ||
|
|
||
| public class AnswerService { | ||
| private final QuestionRepository questionRepository; |
There was a problem hiding this comment.
В AnswerService внедряется QuestionRepository вместо AnswerRepository. Нарушена логика слоев.
No description provided.