-
Notifications
You must be signed in to change notification settings - Fork 101
Lesson 40 queue2 for review #92
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: for-pr
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package com.walking.lesson40_queue2.model; | ||
|
|
||
| public class Task { | ||
| private final String name; | ||
|
|
||
| public Task(String name) { | ||
| this.name = name; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| package com.walking.lesson40_queue2.model; | ||
|
|
||
| public enum TaskStatus { | ||
| ACCEPTED, | ||
| EXECUTED, | ||
| CANCELED; | ||
|
|
||
| @Override | ||
| public String toString() { | ||
| return this.name().toLowerCase(); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,103 @@ | ||
| package com.walking.lesson40_queue2.service; | ||
|
|
||
| import com.walking.lesson40_queue2.model.Task; | ||
| import com.walking.lesson40_queue2.model.TaskStatus; | ||
| import com.walking.lesson40_queue2.util.Logger; | ||
|
|
||
| import java.util.*; | ||
|
|
||
| public class TaskService { | ||
| private final Queue<Task> tasks; | ||
| private final Logger logger; | ||
|
|
||
| public TaskService() { | ||
| this.tasks = new ArrayDeque<>(); | ||
| this.logger = new Logger(); | ||
| } | ||
|
|
||
| public TaskService(Collection<? extends Task> incomingTasks) { | ||
| this.tasks = new ArrayDeque<>(incomingTasks); | ||
| this.logger = new Logger(); | ||
|
|
||
| for (Task incomingTask : incomingTasks) { | ||
| logger.log(getTaskStatusMessage(incomingTask, TaskStatus.ACCEPTED)); | ||
| } | ||
| } | ||
|
|
||
| public List<Task> getAllTasks() { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. У тебя Task содержится в названии класса. Нет нужды дублировать это слово в каждом методе |
||
| return List.copyOf(tasks); | ||
| } | ||
|
|
||
| public boolean acceptSingleTask(Task acceptedTask) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Про Task написал выше. То, что он тут single - понятно из сигнатуры метода:) |
||
| if (tasks.offer(acceptedTask)) { | ||
| logger.log(getTaskStatusMessage(acceptedTask, TaskStatus.ACCEPTED)); | ||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| public boolean acceptAllTasks(Collection<? extends Task> incomingTasks) { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Все же Many, а не All. Я понимаю, что тут ассоциация с addAll у коллекций могла сыграть. Но коллекции и бизнес-логика - они в чутка разных мирах живут:) Вполне хватило бы перегрузки под accept() или acceptMany(). Я предпочитаю последний вариант, но это вкусовщина. acceptAll() - тоже имеет право на жизнь, но выглядит крайне непривычно |
||
| if (tasks.addAll(incomingTasks)) { | ||
| for (Task acceptedTask : incomingTasks) { | ||
| logger.log(getTaskStatusMessage(acceptedTask, TaskStatus.ACCEPTED)); | ||
| } | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| public Task executeSingleTask() { | ||
| Task executedTask = tasks.poll(); | ||
|
|
||
| if (executedTask != null) { | ||
| logger.log(getTaskStatusMessage(executedTask, TaskStatus.EXECUTED)); | ||
| } | ||
|
|
||
| return executedTask; | ||
| } | ||
|
|
||
| public List<Task> executeMultipleTasks(int taskCount) { | ||
| List<Task> executedTasks = new ArrayList<>(); | ||
|
|
||
| for (int i = 0; i < taskCount; i++) { | ||
| executedTasks.add(executeSingleTask()); | ||
| } | ||
|
|
||
| return executedTasks; | ||
| } | ||
|
|
||
| public Task cancelNextTask() { | ||
| Task canceledTask = tasks.poll(); | ||
|
|
||
| if (canceledTask != null) { | ||
| logger.log(getTaskStatusMessage(canceledTask, TaskStatus.CANCELED)); | ||
| } | ||
|
|
||
| return canceledTask; | ||
| } | ||
|
|
||
| public List<Task> cancelMultipleTasks(int taskCount) { | ||
| List<Task> canceledTasks = new ArrayList<>(); | ||
|
|
||
| for (int i = 0; i < taskCount; i++) { | ||
| canceledTasks.add(cancelNextTask()); | ||
| } | ||
|
|
||
| return canceledTasks; | ||
| } | ||
|
|
||
| public Task lookNextTask() { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. и зачем на него смотреть?) Обычно getNext() |
||
| return tasks.peek(); | ||
| } | ||
|
|
||
| public boolean haveTask() { | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. скорее has. Опять же, мб hasNext |
||
| return tasks.peek() != null; | ||
| } | ||
|
|
||
| private String getTaskStatusMessage(Task task, TaskStatus status) { | ||
| return "Task <%s> %s".formatted(task.getName(), status); | ||
| } | ||
| } | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. в целом, нормальное решение. Но очень перегруженный нейминг методов |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,7 @@ | ||
| package com.walking.lesson40_queue2.util; | ||
|
|
||
| public class Logger { | ||
| public void log(String message) { | ||
| System.out.println(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
на будущее - для такого логичнее строковое поле завести. Ибо ключи енама менять зачастую проблематично - особенно, если они в бд храняться. А вот поле можно в любом удобном виде перекручивать, в зависимости от нужд бизнеса
Даже если изначально эти значения взаимовыводимы