Sprint 8 solution time and duration - #4
Conversation
| protected Duration duration = Duration.ZERO; | ||
|
|
||
| /// Дата и время завершения задачи | ||
| protected LocalDateTime endTime; |
There was a problem hiding this comment.
эта переменная нужна только в классе Epic
+
там ей нужны обычные геттер и сеттер
| /// Внесение времени начала выполнения | ||
| public void setStartTime(LocalDateTime startTime) { | ||
| this.startTime = startTime; | ||
| if (startTime != null) { |
There was a problem hiding this comment.
этот код нужно перенести в метод getEndTime()
|
|
||
| /// Параметры CSV файла | ||
| private static final String TABLE_HEADER = "id,type,name,status,description,epic"; | ||
| private static final String TABLE_HEADER = "start, end, duration, id,type,name,status,description,epic"; |
There was a problem hiding this comment.
думаю новые поля нужно добавить в конец, а не начало
There was a problem hiding this comment.
готово
epicId все таки оставил в конце, для читаемости, т.к. там будут пустые ячейки
| } | ||
|
|
||
| return oldEpic; | ||
| if (newEpic.getStartTime() != null) { |
There was a problem hiding this comment.
время в эпике так же обновляется через подзадачи, поэтому этот метод не меняется
| } | ||
|
|
||
| /// Проверка пересечений времени | ||
| private boolean isFoundIntersections(Task taskObject) { |
There was a problem hiding this comment.
первым делом стоит проверить заполнены ли атрибуты времени у taskObject, если нет, можно сразу ответить false и не тратить ресурсы
| private boolean isFoundIntersections(Task taskObject) { | ||
| Task savedTask = getWithoutHistory(taskObject.getId()); | ||
| Task testTask = taskObject.getCopy(); | ||
|
|
There was a problem hiding this comment.
а для чего нужно перекладывать время из taskObject в testTask? почему нельзя передать в findIntersections() сразу taskObject?
There was a problem hiding this comment.
нашел еще одну ошибку, что нельзя проапдейтить нулевое время
ошибку исправил, но перекладывание полностью устранить не удалось
| historyManager.remove(subtaskId); | ||
| } | ||
| idToSubtask.clear(); | ||
| prioritizedTasks.removeIf(task -> task.getType() == Type.EPIC); |
There was a problem hiding this comment.
эпиков там нет, удалить нужно все подзадачи, только лучше это делать в цикле совместно с удалением из истории
| updateEpicsStatus(epic.getId()); | ||
| clearEpicTime(epic); | ||
| } | ||
| prioritizedTasks.removeIf(task -> task.getType() == Type.SUBTASK); |
There was a problem hiding this comment.
лучше удалить в одном цикле с удалением из истории
|
|
||
| /// Обновление продолжительности эпика | ||
| protected void updateEpicTime(Epic epic) { | ||
| if (epic.getId() == null || epic.getStartTime() == null) { |
There was a problem hiding this comment.
отсутствие времени не повод прерывать метод, например у вновь созданного эпика он всегда null
|
|
||
| if (firstSubtask.equals(lastSubtask)) { | ||
| clearEpicTime(epic); | ||
| epic.setStartTime(firstSubtask.getStartTime()); |
There was a problem hiding this comment.
еще нужно присвоить продолжительность и endTime
There was a problem hiding this comment.
заменил этот кусок полностью
No description provided.