Conversation
Fix lesson 39 (queue1) task1
Fix lesson 39 (queue1) task2
| public static void main(String[] args) { | ||
| LinkedList<Integer> integers = new LinkedList<>(); | ||
|
|
||
| integers.reverse(); |
There was a problem hiding this comment.
на пустом списке неинтересно:)
|
|
||
| public E getTop() { | ||
| if (size == 0) { | ||
| throw new ElementNotFoundException(); |
There was a problem hiding this comment.
В общем-то, не зазорно и null тут вернуть. Но это мелочи
| } | ||
|
|
||
| public void delete(Object object) { | ||
| if (top != null) { |
There was a problem hiding this comment.
Вложенность лучше делать минимальной.
if (top == null) {
throw new ElementNotFoundException();
}
// код, который сейчас в ifдаст тот же результат, но читать будет легче
| throw new ElementNotFoundException(); | ||
| } | ||
|
|
||
| public void deleteAllWithEvenHash() { |
There was a problem hiding this comment.
вот тут тонкий момент. Откровенно говоря, общей коллекции и близко не нужен такой метод - он решает какую-то узкую задачу, которая в общей функциональности структуры данных не нужна
Поэтому этот метод логичнее вынести за пределы коллекции. Его будет из-за этого сложнее реализовать. Но это будет намного правильнее с точки зрения дизайна системы
| } | ||
| } | ||
|
|
||
| private boolean isReversible() { |
There was a problem hiding this comment.
не самое удачное название. reversible - это скорее про обратимость, а не разворачиваемость взад-назад:) опять же, мелкое замечание
There was a problem hiding this comment.
Ну и да. В чем проблема развернуть пустой или одноэлементный список?) даже делать ничего не надо
| } | ||
|
|
||
| public void alternativeReverse() { | ||
| if (isReversible()) { |
There was a problem hiding this comment.
опять же, if стоит инвертировать
| this.next = next; | ||
| } | ||
|
|
||
| private boolean hasEvenHash() { |
There was a problem hiding this comment.
тот же момент, что и с методом удаления по четному хэшу. Этой функциональности не место в классе общего назначения
| public String toString() { | ||
| return linkedList.toString(); | ||
| } | ||
| } No newline at end of file |
| if (first.isLast()) { | ||
| last = first; | ||
| } else { | ||
| first.next.previous = first; |
There was a problem hiding this comment.
first.next.previous - чет намудрил. А не поленился бы старый first вынести в отдельную переменную - было бы красиво и просто)
| } | ||
|
|
||
| private void swapFistAndLast() { | ||
| Node<E> temp = first; |
There was a problem hiding this comment.
не уверен, что для этого нужен отдельный метод
| last = temp; | ||
| } | ||
|
|
||
| private void delete(Node<E> current) { |
There was a problem hiding this comment.
deleteNode было бы более понятным
| return value.hashCode() % 2 == 0; | ||
| } | ||
|
|
||
| private void swapNextAndPrevious() { |
There was a problem hiding this comment.
кажется избыточной функциональностью. Но имеет право на жизнь
| previous = temp; | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
замечания из первой задачи во многом актуальны и здесь
| private final LinkedList<E> linkedList; | ||
|
|
||
| public Queue() { | ||
| this.linkedList = new LinkedList<>(); |
There was a problem hiding this comment.
как будто можно это прописать при декларации поля. Но не критично
2 participants
No description provided.