-
Notifications
You must be signed in to change notification settings - Fork 103
For pr #111
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?
For pr #111
Conversation
src/com/walking/lesson17_enum/task3/model/EquiliteralShape.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson14_polymorphism/task1/model/RegularFigure.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson18_instanceof_getClass/task1_getClass/Main.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/lesson38_comparing/task1/service/CarService.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson38_comparing/task2/model/CarIdentifier.java
Outdated
Show resolved
Hide resolved
src/com/walking/lesson38_comparing/task2/service/CarService.java
Outdated
Show resolved
Hide resolved
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
Signed-off-by: AlexeyNiyazov <[email protected]>
| } | ||
|
|
||
| public List<Counter> getCounters() { | ||
| return new ArrayList<>(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.
В целом ок, но я бы использовал неизменяемую коллекцию, чтобы не было разночтений. Иногда другой разработчик - не гений и может решить, что этот метод дает ему оригинальную коллекцию, о которой писал ранее. И, соответственно, карт-бланш на изменение коллекции сервиса. Для таких лучшее средство - сразу ошибку выдавать при попытке изменений, что и делает имьютабельная коллекция:)
| return -1; | ||
| } | ||
|
|
||
| if (number.compareTo(o.getNumber()) == 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.
А есть еще вариант значения number.compareTo(o.getNumber()), если отрицательные и положительные числа уже обработали выше? Зачем этот if?
К слову, зачем несколько раз считать number.compareTo(o.getNumber()), если можно вынести результат в переменную?
| import com.walking.lesson39_queue1.collection.DoublyLinkedList; | ||
|
|
||
| public class Queue<E> { | ||
| private final DoublyLinkedList<E> list = new DoublyLinkedList<>(); |
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 boolean add(Task task) { | ||
| tasks.add(task); | ||
| task.setIndex(tasks.size()); |
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 Node<E> root; | ||
|
|
||
| private int level; | ||
|
|
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.printf("Element %s deleted\n", removed.value); | ||
| return removed; | ||
| } |
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 removed; | ||
| } | ||
|
|
||
| public void straightBypass() { |
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 middleBypass() { |
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.add(current.value); | ||
| } | ||
|
|
||
| public void widthBypass() { |
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.
глагол
| widthBypass(collection, newNodeQueue); | ||
| } | ||
|
|
||
| public class Node<E> implements Comparable<Node<E>> { |
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.
зачем публичная нода? И, в целом, внутренние НЕ статические классы - ред флаг, это потенциальная утечка памяти на ровно месте (ссылка на объект вышестоящего класса в памяти не удаляется, пока есть объекты вложенных)
Исправления в Lesson 14-17
Добавил задания из Lesson 18