Lesson 42 (tree) task1 for review #95
Conversation
| return List.of(task0, task1, task2, task3, task4, task5, task6, task7, task8); | ||
| } | ||
|
|
||
| public static List<Task> getMiddleList() { |
There was a problem hiding this comment.
методы, наверно, приватные должны быть
| System.out.println("Корень дерева: " + tree.getRoot()); | ||
|
|
||
| System.out.println("Содержимое дерева в порядке возрастания ширины (BFS):"); | ||
| tree.breadthFirstSearchTraversal(); |
There was a problem hiding this comment.
никогда раньше не задумывался, как будет обход в ширину на английском. Но тут по смыслу тогда должно быть printBreadthFirstSearch()
| System.out.println(); | ||
|
|
||
| System.out.println("Содержимое дерева в порядке возрастания значений (DFS:inOrder):"); | ||
| List<?> ascendingOrder = tree.getAscendingOrder(); |
There was a problem hiding this comment.
вот такой вариант лучше, чем sout внутри класса дерева. Пусть даже в случае с обходом в ширину возвращался бы List<List<?>>
|
|
||
| public static void print(List<?> list) { | ||
| for (Object object : list) { | ||
| System.out.print(object); |
There was a problem hiding this comment.
Как будто мало чем отличается от sout(list) в таком виде
| private final Comparator<? super E> comparator; | ||
|
|
||
| public BinarySearchTree() { | ||
| comparator = null; |
| } | ||
|
|
||
| private Node<E> getMinNode(Node<E> localRoot) { | ||
| return (localRoot != null && localRoot.hasLeftChild()) ? getMinNode(localRoot.left) : localRoot; |
There was a problem hiding this comment.
лишние скобки вокруг условия. Если без них кажется, что сливается тернарка - просто разнеси ее на разные строки, например, как ниже. Ибо если сливается без скобок - со скобками тоже будет сливаться:))
return localRoot != null && localRoot.hasLeftChild()
? getMinNode(localRoot.left)
: localRoot;
| if (comparator != null) { | ||
| return comparator.compare(keyValue, currentValue); | ||
| } else { | ||
| Comparable<E> comparable = (Comparable<E>) keyValue; |
There was a problem hiding this comment.
избыточное выделение переменной, на мой взгляд
|
|
||
| int route = getRouteByKey(key, localRoot.value); | ||
|
|
||
| return isSearchHit(route) ? |
There was a problem hiding this comment.
Не стоит делать вложенные тернарки. Тяжело читать, тяжело поддерживать, тяжело снимать порчу, наведенную коллегами
| } | ||
|
|
||
| public void put(E key) { | ||
| root = put(key, root); |
There was a problem hiding this comment.
делать приватную перегрузку публичного метода - обычно так себе тема, на грани фола. Лучше приватный метод назвать иначе - аля putIntetnal()
Именно в твоем случае выглядит прилично, на мой взгляд, но в целом, особенно в классах логики, такого стоит избегать
| public List<E> getAscendingOrder() { | ||
| List<E> list = new ArrayList<>(); | ||
|
|
||
| inOrderToList(root, list); |
| List<E> inorder = getInOrderList(); | ||
| List<E> middles = new ArrayList<>(inorder.size()); | ||
|
|
||
| middleExtraction(inorder, middles); |
|
|
||
| middleExtraction(inorder, middles); | ||
|
|
||
| BinarySearchTree<E> balanced = new BinarySearchTree<E>(this.comparator); |
There was a problem hiding this comment.
как будто можно и без создания нового дерева обойтись - тебе же достаточно ноды, класс дерева - просто обертка над ним
|
|
||
| int route = getRouteByKey(key, localRoot.value); | ||
|
|
||
| return isSearchHit(route) ? |
| } | ||
|
|
||
| private Node<E> removeFound(Node<E> found, Node<E> parent) { | ||
| Node<E> newborn = found.hasTwoChild() ? removeSuccessor(found, parent) : getFoundChild(found); |
There was a problem hiding this comment.
лучше разнести ауты на отдельные строки
| return result <= 0; | ||
| } | ||
|
|
||
| private boolean isSearchHit(int route) { |
There was a problem hiding this comment.
на мой взгляд, чутка перемудрил с getRouteByKey() и isSearchHit()
В первом случае не очевиден смысл - хотя это просто compare/compareTo для нод, во втором - просто бесполезный метод:)
| * <p> | ||
| * Итеративные алгоритмы поиска, добавления и удаления. | ||
| */ | ||
| public class BinarySearchTree_v2<E> { |
There was a problem hiding this comment.
постараюсь в следующий раз посмотреть. Сейчас морально не готов разбираться в дереве с soft-delete:)
| private int getRouteByKey(E keyValue, E currentValue) { | ||
| if (comparator != null) { | ||
| return comparator.compare(keyValue, currentValue); | ||
| } else { |
There was a problem hiding this comment.
Забыл в первой версии добавить. else здесь не нужен - попадание в if выше уже гарантирует, что до этого кода не дойдет, т.к. в if'е есть return
2 participants
No description provided.