Conversation
Binary-Cat-01
changed the title
|
|
||
| @Override | ||
| public String toString() { | ||
| return this.name().toLowerCase(); |
There was a problem hiding this comment.
на будущее - для такого логичнее строковое поле завести. Ибо ключи енама менять зачастую проблематично - особенно, если они в бд храняться. А вот поле можно в любом удобном виде перекручивать, в зависимости от нужд бизнеса
Даже если изначально эти значения взаимовыводимы
| } | ||
| } | ||
|
|
||
| public List<Task> getAllTasks() { |
There was a problem hiding this comment.
У тебя Task содержится в названии класса. Нет нужды дублировать это слово в каждом методе
| return List.copyOf(tasks); | ||
| } | ||
|
|
||
| public boolean acceptSingleTask(Task acceptedTask) { |
There was a problem hiding this comment.
Про Task написал выше. То, что он тут single - понятно из сигнатуры метода:)
| return false; | ||
| } | ||
|
|
||
| public boolean acceptAllTasks(Collection<? extends Task> incomingTasks) { |
There was a problem hiding this comment.
Все же Many, а не All. Я понимаю, что тут ассоциация с addAll у коллекций могла сыграть. Но коллекции и бизнес-логика - они в чутка разных мирах живут:)
Вполне хватило бы перегрузки под accept() или acceptMany(). Я предпочитаю последний вариант, но это вкусовщина. acceptAll() - тоже имеет право на жизнь, но выглядит крайне непривычно
| return canceledTasks; | ||
| } | ||
|
|
||
| public Task lookNextTask() { |
There was a problem hiding this comment.
и зачем на него смотреть?) Обычно getNext()
| return tasks.peek(); | ||
| } | ||
|
|
||
| public boolean haveTask() { |
There was a problem hiding this comment.
скорее has. Опять же, мб hasNext
| private String getTaskStatusMessage(Task task, TaskStatus status) { | ||
| return "Task <%s> %s".formatted(task.getName(), status); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
в целом, нормальное решение. Но очень перегруженный нейминг методов
2 participants
No description provided.