Sprint 8 solution time and duration #3
Conversation
avfyodorov
left a comment
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый день, Максим!
Практически всё сделано, единственное нужно будет немного поправить работу с отсортированным по времени списком.
src/model/Epic.java
Outdated
| public class Epic extends Task { | ||
| private ArrayList<Subtask> subtaskList = new ArrayList<>(); | ||
| private Duration duration; | ||
| private LocalDateTime startTime; |
There was a problem hiding this comment.
private Duration duration;
private LocalDateTime startTime;Эти два поля лишние, потому что они дублируют соответствующие поля в классе предке - Task.
Нужно использовать поля класса Task
Поля в Task лучше сделать protected, чтобы они были доступны напрямую.
| @@ -0,0 +1,7 @@ | |||
| package manager; | |||
There was a problem hiding this comment.
Собственные исключения принято хранить в отдельном пакете, например, exceptions
| private int getNextID() { | ||
| return nextID++; | ||
| @Override | ||
| public List<Task> getPrioritizedTasks() { |
There was a problem hiding this comment.
К сожалению, данная реализация не соответствует требованиям ТЗ:
.=============
Предполагается, что пользователь будет часто запрашивать этот список, поэтому подберите подходящую структуру данных для хранения. Сложность получения должна быть уменьшена ... до O(n)
...
Если сортировать список заново каждый раз, сложность получения будет O(n*log(n)). Можно хранить все задачи заранее отсортированными с помощью класса TreeSet.
.===========
Метод получения списка отсортированных задач должен выглядеть так:
@Override
public List<Task> getPrioritizedTasks() {
return new ArrayList<>(prioritizedTasks);
}Нужно будет завести переменную - отсортированный список и поддерживать её в актуальном состоянии.
protected final Set<Task> prioritizedTasks = new TreeSet<>(Comparator.comparing(Task::getStartTime));
avfyodorov
left a comment
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый вечер, Максим!
Ещё несколько небольших уточнений.
И ставьте, пожалуйста, пометки, чтобы было понятно, что комментарии замечены. )
src/manager/InMemoryTaskManager.java
Outdated
| @Override | ||
| public void deleteTasks() { | ||
| tasks.clear(); | ||
| prioritizedTasks.clear(); |
There was a problem hiding this comment.
Не совсем так. Лучше как-то так -
@Override
public void deleteTasks() {
for (Task task : tasks.values()) {
historyManager.remove(task.getId());
prioritizedTasks.remove(task);
}
tasks.clear();
}
There was a problem hiding this comment.
Все комментарии замечены, все исправил
Надеюсь правильно
src/manager/InMemoryTaskManager.java
Outdated
| public void deleteEpics() { | ||
| epics.clear(); | ||
| subtasks.clear(); | ||
| prioritizedTasks.clear(); |
There was a problem hiding this comment.
И здесь чистить из истории и из отсортированного списка, но ТОЛЬКО ПОДЗАДАЧИ.
src/manager/InMemoryTaskManager.java
Outdated
| @Override | ||
| public void deleteSubtasks() { | ||
| subtasks.clear(); | ||
| prioritizedTasks.clear(); |
| tasks.remove(id); | ||
| Task task = tasks.remove(id); | ||
| if (task != null) { | ||
| prioritizedTasks.remove(task); |
src/manager/InMemoryTaskManager.java
Outdated
| subtasks.remove(subtask.getId()); | ||
| Epic epic = epics.get(id); | ||
| if (epic != null) { | ||
| ArrayList<Subtask> epicSubtasks = epic.getSubtaskList(); |
| int epicID = subtask.getEpicID(); | ||
| subtasks.remove(id); | ||
| prioritizedTasks.remove(subtask); | ||
| historyManager.remove(id); |
avfyodorov
left a comment
avfyodorov
left a comment
There was a problem hiding this comment.
Добрый день, Максим!
Замечаний нет.
Работа принята.
No description provided.