MyTreeSet #12
MyTreeSet #12
Conversation
| xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd"> | ||
| <modelVersion>4.0.0</modelVersion> | ||
|
|
||
| <groupId>ru,spbau.mit.kazakov.MyTreeSet</groupId> |
There was a problem hiding this comment.
Из-за запятой после ru ругается на неправильный groupId и не собирается из консоли
| * therefore generic type must implements Comparable interface. | ||
| */ | ||
| public MyTreeSet() { | ||
| comparator = (o1, o2) -> ((Comparable<E>) o1).compareTo(o2); |
There was a problem hiding this comment.
Имеет смысл заглушить предупреждение, //noinspection unchecked
| */ | ||
| @Override | ||
| public boolean contains(@NotNull Object value) { | ||
| return find((E) value) != null; |
There was a problem hiding this comment.
Тут как раз идея в том, что значение, которое мы ищем contains, может вообще не быть типа E, просто компаратор должен говорить, что значения равны. То есть find по идее тоже должен принимать Object и не делать никаких предположений о типах сравниваемых объектов. Хотя из-за стирания работать должно и так.
There was a problem hiding this comment.
@yurii-litvinov но компаратор же у меня такой: Comparator<? super E> comparator, т.е. ему не получится подсунуть Object. Или тут нужно не спускаться по дереву, а работать за линию, проверяя на equals все элементы?
| /** | ||
| * Node for binary search tree. Stores left, right children and value. | ||
| */ | ||
| private class Node { |
There was a problem hiding this comment.
static? Параметр-тип, правда, придётся писать явно, но оно того стоит обычно.
There was a problem hiding this comment.
@yurii-litvinov Я правильно понимаю, что в таком варианте, везде, где я пишу node.value, мне нужно дописывать каст к E?
| } | ||
|
|
||
| E prev = next; | ||
| next = higher(next); |
There was a problem hiding this comment.
next() за примерно логарифм? Можно же за линейное время.
| TreeSet<Integer> random = new TreeSet<>(); | ||
|
|
||
| for (int i = 0; i < 100; i++) { | ||
| Integer valueToAdd = ThreadLocalRandom.current().nextInt(0, 30); |
There was a problem hiding this comment.
Использовать случайные данные в тестах --- плохая идея. Если он не проходит на одном наборе данных из десяти тысяч, кому-то, кто будет разбираться. почему билд иногда валится, я бы не позавидовал.
There was a problem hiding this comment.
@yurii-litvinov а если я буду использовать какой-нибудь сид?
| mySet.remove('t'); | ||
|
|
||
| assertArrayEquals(new Character[]{'v', 'z'}, mySet.toArray()); | ||
| } |
There was a problem hiding this comment.
Судя по coverage, нет теста на самый интересный случай удаления --- когда у удаляемого узла есть оба сына (причём, тестить надо на случай, когда они нетривиальные поддеревья, с этим часто бывают проблемы).
| next = higher(next); | ||
| return prev; | ||
| } | ||
| } |
There was a problem hiding this comment.
Кстати, было бы круто, если бы итератор инвалидировался при модификации дерева.
No description provided.