-
Notifications
You must be signed in to change notification settings - Fork 103
lesson14 task1 #110
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?
lesson14 task1 #110
Conversation
src/com/walking/lesson16_abstract_class_interface/task2/Hello.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task2/Main.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task2/Printer.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task3/Animal.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task3/Cat.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task2/printer/Goodbye.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task2/printer/Printer.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson16_abstract_class_interface/task2/Main.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson32_files_1/task3/repository/CarRepository.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task1/model/Counter.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task1/model/Counter.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task1/service/CounterService.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task1/service/CounterService.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task1/service/CounterService.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task1/service/CounterService.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task2/collection/MyCollection.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson37_collection_list/task2/collection/MyCollection.java
Outdated
Show resolved
Hide resolved
| System.out.println(newList); | ||
| newList.reverse(); | ||
| System.out.println(newList); | ||
| newList.removeEvenHashCodes(); |
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.
Вот тут есть нюанс. Коллекция - это коллекция. Ей в пень не вперся функционал удаления элементов с четным хешкодом. Значит, если это все же нужно, стоит сделать это отдельно
На самом деле тут и моя вина есть - не стоило давать такую задачку до знакомства с итератором. Без него или же метода получения следующего элемента, красиво это не сделаешь. Но на будущее стоит отложить: у каждого класса должна быть своя зона ответственности, накручивать дополнительную функциональность ради сиюминутной потребности - плохая затея
| @@ -0,0 +1,7 @@ | |||
| package com.walking.lesson39_queue1.task1.exception; | |||
|
|
|||
| public class ListIsEmptyException extends RuntimeException { | |||
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.
EmptyListException звучит более органично. Правда, не уверен, что это должно быть исключением - пустой и пустой, чего ругаться?)
| return size == 0; | ||
| } | ||
|
|
||
| public boolean find(Object o) { |
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.
это скорее contains(). Семантика find() обычно подразумевает возвращение найденного элемента. Именно здесь это постольку-поскольку актуально, но даже в таком кастрированном виде может быть, что equals() определен не по всем полям и на вход поступает лишь частично инициированный объект. Тогда зона применения метода будет лежать в получении объекта коллекции, в надежде найти значения второстепенных атрибутов. Надеюсь, не слишком сложно описал
|
|
||
| public Object pop() { | ||
| Object del = list.getTailValue(); | ||
| list.remove(del); |
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 remove(Object o) { | ||
| if (isEmpty()) { | ||
| throw new ListIsEmptyException("Список пуст."); |
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.
вообще - довольно показательная ошибка с избыточными исключениями. Если я удаляю несуществующий элемент из непустой коллекции - это не ошибка. Но если коллекция пуста - сюрприз. Это вводит неочевидное и двойственное поведение. Если когда-то придется реализовывать свои структуры данных - такого стоит избегать, потому что это буквально лишает АПИ прозрачности для пользователя
|
|
||
| Node<E> current = tail; | ||
| while (current.prev != null) { | ||
| if (current.prev.value.equals(el)) { |
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.
выглядит, будто current.prev просится в переменную для части использований
| return false; | ||
| } | ||
|
|
||
| public int findAll(E el) { |
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.
опять же, неожиданное поведение для find*-метода
| 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.
хорошая реализация. Ошибки, унаследованные от однонаправленного списка, дублировать не стал
| import java.util.LinkedList; | ||
|
|
||
| public class TaskService { | ||
| private final Deque<Task> tasks; |
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.
Выглядит, будто здесь можно было и Queue использовать как интерфейс
No description provided.