-
Notifications
You must be signed in to change notification settings - Fork 23
Евгений Кузин, Trie #15
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: master
Are you sure you want to change the base?
Conversation
|
|
||
| static class TrieNode { | ||
| Map<Character, TrieNode> children = new TreeMap<>(); | ||
| boolean leaf; |
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.
Лист - это узел с нулевым количеством дочерних узлов. То есть лист можно определить по пустому children. Зачем в таком случае нужна отдельная переменная - непонятно. В Trie нужно отмечать, соответствует ли узел какому-либо слову, или просто является промежуточным, но тогда и название у переменной должно быть другое.
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.
не понял, нужно изменить название переменной leaf на другое или удалить эту переменную вообще?
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.
Я не понял, зачем нужна эта переменная. Если она обозначает лист (узел без дочерних узлов), то её нужно удалить, потому что узел можно легко вычислить с помощью переменной children. Если она обозначает что-то другое, то её нужно переименовать.
| } | ||
|
|
||
| TrieNode root = new TrieNode(); | ||
| List existedChars = new ArrayList<Character>(); |
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 элементов.
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.
Зачем вообще нужна переменная existedChars? Имя не очень помогает понять её смысл.
| public void insert(String str) { | ||
| int i = 0; | ||
| TrieNode v = root; | ||
| boolean bro = false; |
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.
bro? Не очень понятное имя переменной.
| TrieNode v = root; | ||
| boolean bro = false; | ||
| for (char ch : str.toLowerCase().toCharArray()) { | ||
| i++; |
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.
Зачем вам нужна переменная i, если она нигде не используется?
| if (!v.children.containsKey(ch)){ | ||
| v.children.put(ch, new TrieNode()); | ||
| } | ||
| else { |
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.
Можно писать без фигурной скобки и на одной строке, т.е. else if.
| } | ||
| } | ||
| return result; | ||
| } |
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.
Не хватает поиска всех строк в дереве с заданным префиксом - это было в задании. Кроме того, нужно написать стандартные методы, такие как toString, equals, hashCode.
| TrieNode v = root; | ||
| boolean broExist = false; | ||
| String str2 = str; | ||
| System.out.println(str2); |
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.
Используйте логирование вместо println. Логирование вы всегда можете отключить одной командой, а println придется вручную убирать из кода.
| for (char ch : str.toLowerCase().toCharArray()) { | ||
| if ( v != root && !existedChars.contains(ch) ) { | ||
| System.out.println(v.children.keySet()); | ||
| //v.children.remove(ch); |
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.
Не оставляйте комментарии в коде. Если понадобится вернуться к предыдущему варианту, то все старые версии вашего кода, которые вы коммитили, есть в гите и на GitHub-е.
| if ( v != root && !existedChars.contains(ch) ) { | ||
| System.out.println(v.children.keySet()); | ||
| //v.children.remove(ch); | ||
| v.children.put('@', v.children.remove(ch)); |
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.
Что значит @?
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 org.apache.logging.log4j.LogManager; | ||
| import org.apache.logging.log4j.Logger; | ||
|
|
||
| import javax.xml.crypto.Data; |
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.
Лишний импорт. Он вам в коде точно не нужен, лучше его удалить.
| } | ||
|
|
||
| TrieNode root = new TrieNode(); | ||
|
|
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.children.get(ch).brunch = true; | ||
| } | ||
|
|
||
| } else if (node.brunch) node.brunch = false; |
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.
Если ставите фигурные скобки у одной ветви if-а, то ставьте и у другой.
|
|
||
| searchHelp(word, prefix, node); | ||
|
|
||
| System.out.println("_________________"); |
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.
Если в описании метода не сказано, что он должен выводить что-то на экран, то не стоит писать в нем println. Метод должен возвращать полученное значение, а не выводить на экран. Исключение - логирование.
| static class TrieNode { | ||
| Map<Character, TrieNode> children = new TreeMap<>(); | ||
| boolean leaf; | ||
| boolean brunch; |
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.
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.
упс
|
|
||
| @Test | ||
| void exampleTest() { | ||
| logger.info("Test started"); |
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.
Не все методы протестированы.
|
Не реализованы стандартные методы, некоторые старые замечания ещё не исправлены. Пока что 3. |

No description provided.