Conversation
* src/manager/InMemoryHistoryManager.java
+ Реализован двусвязный список + HashMap<id,Node>
+ add(task) теперь удаляет дубликаты через removeNode за O(1)
+ remove(id) — новый метод HistoryManager
* src/manager/TaskManager.java
+ в интерфейс добавлен void remove(int id)
* src/manager/InMemoryTaskManager.java
+ вызовы historyManager.remove(...) в removeTask / removeEpic / removeSubtask
* src/manager/HistoryManager.java
+ объявлен метод remove(int id)
* tests
+ HistoryManagerTest — проверяет отсутствие дублей и порядок
+ InMemoryTaskManagerTest — проверка очистки истории при удалении,
истории > 10, дубль-просмотров
+ TaskManagerHistoryIntegrationTest — интеграция: удаление эпика убирает
эпик и все Subtask'и из истории
Issue: sprint-6 / ТЗ «неограниченная история без дублей»
… методах, сохранение исходных id
…restore (если восстановление)
…а O(1); чистка импортов
…обы убрать предупреждения
… minimal IDEA files
PrioritizedViewTest:
LexLippi
left a comment
There was a problem hiding this comment.
Привет!
Неплохая работа!
Есть некоторое количество замечаний, которые необходимо исправить!
Желаю удачи!
| // статус | ||
| if (subs.isEmpty()) { | ||
| epic.setStatus(Status.NEW); | ||
| } else { | ||
| boolean allNew = subs.stream().allMatch(s -> s.getStatus() == Status.NEW); | ||
| boolean allDone = subs.stream().allMatch(s -> s.getStatus() == Status.DONE); | ||
| if (allNew) { | ||
| epic.setStatus(Status.NEW); | ||
| } else if (allDone) { | ||
| epic.setStatus(Status.DONE); | ||
| } else { | ||
| epic.setStatus(Status.IN_PROGRESS); | ||
| } | ||
| } | ||
|
|
||
| // duration = сумма минут (null считаем как 0) | ||
| long minutes = | ||
| subs.stream() | ||
| .map(Subtask::getDuration) | ||
| .filter(Objects::nonNull) | ||
| .mapToLong(Duration::toMinutes) | ||
| .sum(); | ||
| epic.setCalculatedDuration(minutes == 0 ? null : Duration.ofMinutes(minutes)); | ||
|
|
||
| // start = минимальный start subtask; end = макс end | ||
| LocalDateTime start = | ||
| subs.stream() | ||
| .map(Subtask::getStartTime) | ||
| .filter(Objects::nonNull) | ||
| .min(LocalDateTime::compareTo) | ||
| .orElse(null); | ||
|
|
||
| LocalDateTime end = | ||
| subs.stream() | ||
| .map(Subtask::getEndTime) | ||
| .filter(Objects::nonNull) | ||
| .max(LocalDateTime::compareTo) | ||
| .orElse(null); |
There was a problem hiding this comment.
Делаем 5 пробеганий по коллекции подзадач, кажется, что все эти действия можно выполнить за один обход
| /* ───────────── CSV утилиты ───────────── */ | ||
|
|
||
| private static Task fromCsv(String csv) { | ||
| String[] p = csv.split(",", -1); |
There was a problem hiding this comment.
лучше не называть переменные одним символом. taskParts вполне подойдет
| } | ||
| } | ||
|
|
||
| private static LocalDateTime parseTimeOrNull(String s) { |
There was a problem hiding this comment.
Это не имеет отношения к FileBackedTaskManager, поэтому лучше унести в отдельный класс с аналогичными утилитами
| int maxId = 0; | ||
| for (Task task : tasks) { | ||
| maxId = Math.max(maxId, task.getId()); | ||
| } | ||
| for (Epic epic : epics) { | ||
| maxId = Math.max(maxId, epic.getId()); | ||
| } | ||
| for (Subtask subtask : subtasks) { | ||
| maxId = Math.max(maxId, subtask.getId()); | ||
| } |
There was a problem hiding this comment.
Зачем делать второй проход по коллекциям, если идентификаторы не меняем?
| public void setCalculatedDuration(Duration duration) { | ||
| this.duration = duration; | ||
| } | ||
|
|
||
| public void setCalculatedStart(LocalDateTime start) { | ||
| this.startTime = start; | ||
| } | ||
|
|
||
| public void setCalculatedEnd(LocalDateTime end) { | ||
| this.endTime = end; | ||
| } |
There was a problem hiding this comment.
Эти методы лучше не делать public иначе их может нечаянно вызвать в коде другой программист, что приведет к нарушению логики работы.
Поэтому публичные методы должны быть действительно публичными
| private static String escape(String s) { | ||
| return s == null ? "" : s; | ||
| } |
There was a problem hiding this comment.
Метод лучше вынести в отдельный класс с утилитами
| private static String escape(String s) { | ||
| return s == null ? "" : s; | ||
| } |
There was a problem hiding this comment.
Аналогично про этот метод и не придется его дублировать
LexLippi
left a comment
There was a problem hiding this comment.
Привет!
Хорошая работа!
Желаю удачи в следующих спринтах!
Спасибо за ревью! Исправил замечания:
Пересчёт эпика за один проход:
– добавлен Epic.recalcFromSubtasks(List),
– InMemoryTaskManager.recalcEpic(...) теперь вызывает пересчёт эпика (без 5 проходов).
Нейминг:
– в FileBackedTaskManager.fromCsv(...) p → taskParts.
Утилиты:
– parseTimeOrNull и escape вынесены в util.CsvUtils;
– дублирующие escape(...) в моделях удалены; в Task/Epic/Subtask теперь CsvUtils.escape(...).
Восстановление из файла:
– maxId считаю на лету при чтении строк CSV (без дополнительных проходов по коллекциям).
Расчётные поля Epic:
– публичные сеттеры расчётных полей убраны; вместо этого публичный recalcFromSubtasks(...).
Дополнительно:
– добавлены TODO-комментарии в местах правок по ревью.
Что сделано по ТЗ
Модель
Task
добавлены поля Duration duration и LocalDateTime startTime;
вычисляемый getEndTime() как startTime + duration;
toCsvRow() расширен—добавлены колонки durationMinutes и startTime;
equals/hashCode/toString учитывают новые поля.
Subtask
наследует duration/startTime/getEndTime;
хранит epicId; корректный экспорт в CSV.
Epic
расчётные поля: duration, startTime, endTime считаются от подзадач;
пакетные сеттеры setCalculatedDuration/Start/End для пересчёта менеджером;
хранит subtaskIds.
Менеджеры
InMemoryTaskManager
приоритизация через TreeSet по startTime (доп. сравнение по id);
в приоритизированный список не попадают эпики и задачи без startTime (по ТЗ);
валидация пересечений при add/update задач и подзадач; пересечение определяется как наложение интервалов [start, end), касание по границе допустимо;
пересчёт эпика:
status (все NEW → NEW; все DONE → DONE; иначе IN_PROGRESS),
duration = сумма минут подзадач,
start = минимальный startTime подзадач, end = максимальный endTime;
методы для восстановления из файла: putTask/Epic/SubtaskPreserveId и setNextIdAfterRestore.
InMemoryHistoryManager
история просмотров как LinkedHashMap с дедупликацией;
лимит 10 элементов; поддержка remove(id).
Файловый менеджер
FileBackedTaskManager
сохранение в CSV в расширенном формате:
id,type,name,status,description,durationMinutes,startTime,epic;
обратная совместимость чтения старого формата (6 колонок из спринта 7);
восстановление: чтение всех строк → разбор в Task/Epic/Subtask → put*PreserveId → setNextIdAfterRestore;
автосохранение на всех add/update/remove.
формат времени единый — yyyy-MM-dd HH:mm (как в Task.CSV_TIME_FMT).
Тесты
TaskManagerTest — общие сценарии:
правила статуса эпика (без сабтасков, все NEW, все DONE, смешанные/IN_PROGRESS);
приоритизация + запрет пересечений;
агрегирование времени эпика (start, end, duration).
InMemoryTaskManagerTest и FileBackedTaskManagerTest — проверки для обеих реализаций (вторая работает с temp-файлом и корректно чистит его).
InMemoryHistoryManagerTest — дедуп, лимит 10, удаление.
EpicStatusTest — быстрый санити-тест правил статуса на обеих реализациях.
Нефункциональные
привёл код к Google Java Style (фигурные скобки в if/while/for, отступы, переносы);
устранил/подавил безопасные IDE-варнинги (@SuppressWarnings там, где метод нужен публично, но прямо сейчас не вызывается; аккуратный delete() в тестах на Windows).
Формат CSV (итог)
id,type,name,status,description,durationMinutes,startTime,epic
durationMinutes — целое число минут или пусто;
startTime — yyyy-MM-dd HH:mm или пусто;
у Task/Epic поле epic пустое; у Subtask — epicId.
Ограничения/договорённости
эпики не включаются в getPrioritizedTasks();
задачи без startTime не участвуют в приоритизации;
касание интервалов по границе не считается пересечением.
Как проверить
запустить тесты из IDE ;
руками: создать задачи/подзадачи с startTime и duration, убедиться в сортировке, запрете пересечений и корректности истории;
для файлового менеджера: создать/перезапустить — данные сохраняются/восстанавливаются из CSV