-
Notifications
You must be signed in to change notification settings - Fork 103
Lesson_37 (collection list) - for review. #88
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?
Lesson_37 (collection list) - for review. #88
Conversation
|
|
||
| public static void main(String[] args) { | ||
| CounterService counterService = new CounterService(); | ||
| counterService.displayContent(); |
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.
замечание то же, что и ранее: сервис не должен отвечать за способ вывода
|
|
||
| Counter[] allCounters = new Counter[counterService.getAllCounters().size() + 5]; | ||
|
|
||
| counterService.getAllCounters().toArray(allCounters); |
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.
предсоздавать массив в данном случае не обязательно
| System.out.println("Названия счетчиков в массиве:"); | ||
|
|
||
| for (Counter counter : counters) { | ||
| if (counter == null) { |
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.
тернарка смотрелась бы лаконичнее
|
|
||
| private final ArrayList<Counter> counters; | ||
|
|
||
| public CounterService(Counter... counters) { |
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.
Вполне можно и параметром принимать лист. Опционально
| } | ||
|
|
||
| public List<Counter> getAllCounters() { | ||
| return counters; |
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.
на будущее - зачастую хорошей практикой принимается отдавать копию коллекции - на случай, если необходимо ограничить бесконтрольное изменение такого хранилища. Тогда добавление и иные изменения коллекции ограничены апи управляющего класса (здесь - сервиса), а на чтение всегда будет отдаваться копия, изменение которой не повлияет на основную коллекцию
| } | ||
|
|
||
| public Counter getFirst() { | ||
| return counters.get(0); |
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.
с 21 джавы можно проще
| return this.counters.addAll(candidates); | ||
| } | ||
|
|
||
| public boolean removeIfNotMatch(Collection<? extends Counter> sample) { |
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.
чтобы потыкать функциональность листов - норм, но в целом такая семантика вызывает вопросы
| List<Counter> uniques = new ArrayList<>(); | ||
|
|
||
| for (Counter counter : counters) { | ||
| if (!uniques.contains(counter)) { |
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.
у тебя же вроде не переопределен equals(). Будет криво работать
| return uniques; | ||
| } | ||
|
|
||
| public boolean removeAllZeroValue() { |
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.
непонятное название, смысл становится прозрачным только после погружения в тело метода
| return true; | ||
| } | ||
|
|
||
| private List<Counter> getZeroValues() { |
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.
советую располагать методы в соответствие с модификатором доступа - сначала публичные, потом протектед и т.д.
| public Counter increaseCounter(String name, int value) { | ||
| Counter counter = getCounterByName(name); | ||
|
|
||
| if (counter == null) { |
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.
тернарный оператор?
| public Counter decreaseCounter(String name, int value) { | ||
| Counter counter = getCounterByName(name); | ||
|
|
||
| if (counter == null) { |
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.
и здесь
| public Counter incrementCounter(String name) { | ||
| Counter counter = getCounterByName(name); | ||
|
|
||
| if (counter == null) { |
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.
.
| public void displayContent() { | ||
| System.out.printf("Всего счетчиков: %s\n%s\n", counters.size(), "-".repeat(20)); | ||
|
|
||
| if (counters.isEmpty()) { |
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.
отписывал выше
| this.next = next; | ||
| } | ||
| } | ||
| } |
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.
хорошее решение
No description provided.