Skip to content

Popkovdmitry_Quest#15

Open
DmitriyPopkov wants to merge 11 commits intodemologin:mainfrom
DmitriyPopkov:popkovdmitry
Open

Popkovdmitry_Quest#15
DmitriyPopkov wants to merge 11 commits intodemologin:mainfrom
DmitriyPopkov:popkovdmitry

Conversation

@DmitriyPopkov
Copy link

No description provided.

Copy link
Owner

@demologin demologin left a comment

Choose a reason for hiding this comment

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

Общий вывод по проекту

Проект демонстрирует хорошее понимание основ Java и умение работать с внешними библиотеками (Apache Commons CSV). Использование современных конструкций языка, таких как switch expression, говорит о стремлении следить за развитием технологий.

Однако, ключевой областью для роста является понимание жизненного цикла сервлетов. Запомните: сервлеты не должны хранить состояние пользователя в своих полях, так как один экземпляр сервлета обрабатывает запросы от всех пользователей одновременно. Все данные пользователя должны находиться в HttpSession или HttpServletRequest. Также стоит уделить внимание обработке исключений и заменить System.out.println на полноценное логирование.

Рекомендации:

  • Продолжить углубление знаний по паттернам проектирования
  • Изучить асинхронное программирование более глубоко
  • Уделить внимание документированию кода

Итоговая оценка: B


public class Repository {

public final static String PATH = "/data.csv";
Copy link
Owner

Choose a reason for hiding this comment

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

Константа PATH содержит жестко закодированный путь. Рекомендуется использовать конфигурационные файлы или переменные окружения для гибкости настройки. [WARNING]

recordsList.add(values);
}
} catch (IOException e) {
throw new RuntimeException(e);
Copy link
Owner

Choose a reason for hiding this comment

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

Использование throw new RuntimeException(e) скрывает детали ошибки. Следует использовать логирование (например, SLF4J) и выбрасывать специфичные исключения. [ERROR]

public boolean equals(Object o) {
if (o == null || getClass() != o.getClass()) return false;
Question question1 = (Question) o;
for (int i = 0; i < question1.getAnswersId().size(); i++) {
Copy link
Owner

Choose a reason for hiding this comment

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

Ручная реализация сравнения списков в equals избыточна. Objects.equals(answersId, question1.answersId) выполнит ту же работу эффективнее и чище. [WARNING]

if (!this.getAnswersId().get(i).equals(question1.getAnswersId().get(i))) {
return false;
}
} catch (ArrayIndexOutOfBoundsException e) {
Copy link
Owner

Choose a reason for hiding this comment

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

Перехват ArrayIndexOutOfBoundsException внутри equals — это антипаттерн. Логику сравнения следует строить на проверке размеров коллекций. [ERROR]

try {
question.setQuantityAnswers(Integer.parseInt(record[2]));
} catch (NumberFormatException e) {
System.out.println("invalid quantity answer: " + record[2]);
Copy link
Owner

Choose a reason for hiding this comment

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

Использование System.out.println для логирования недопустимо в профессиональной разработке. Следует использовать Logger. [ERROR]

}

List<Question> fillQuestionsFromRepo(List<String[]> repoList) {
questions = new ArrayList<>();
Copy link
Owner

Choose a reason for hiding this comment

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

Метод fillQuestionsFromRepo изменяет состояние поля класса и одновременно возвращает его. Лучше сделать метод чистым (только возврат списка). [WARNING]

package repository;

import org.apache.commons.csv.CSVFormat;
import org.apache.commons.csv.CSVRecord;
Copy link
Owner

Choose a reason for hiding this comment

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

Импорт CSVRecord и использование Apache Commons CSV — хорошее решение для работы с данными, это упрощает парсинг. [INFO]

public class Question {
String id;
String question;
int quantityAnswers;
Copy link
Owner

Choose a reason for hiding this comment

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

Названия полей должны соответствовать Java Naming Conventions. 'quantityAnswers' — ок, но 'answersId' лучше заменить на 'answerIds'. [INFO]

introStop = record[1];
continue;
}
if (record.length != 3 && record.length != 4) {
Copy link
Owner

Choose a reason for hiding this comment

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

Сложная логика валидации длины записи (3 или 4). Стоит выделить это в отдельный метод-валидатор для соблюдения принципа Single Responsibility. [WARNING]


} else this.error = "Error";

Cookie cookie = new Cookie("someChek", chek);
Copy link
Owner

Choose a reason for hiding this comment

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

Создание Cookie без флага HttpOnly делает их уязвимыми для XSS атак. Рекомендуется 'cookie.setHttpOnly(true)'. [WARNING]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants