Interactive mode for avl tree #57
Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Почти всё хорошо, но надо немного поправить
There was a problem hiding this comment.
Лучше хранить либо просто height, либо только balance (что сложнее). Потому что вообще-то, красно-чёрные деревья круче АВЛ в частности тем, что там для цвета достаточно одного бита, а для баланса в АВЛ надо два. "А давайте сделаем три (!) тридцатидвухбитные переменные для хранения баланса", решили Вы :) Адельсон-Вельский бы не одобрил.
There was a problem hiding this comment.
В этой задаче ключ уже должен быть строковым по условию
There was a problem hiding this comment.
| while(root != NULL) | |
| while (root != NULL) |
There was a problem hiding this comment.
Какая-то очень умная функция, смотрит на root, иногда выставляет значение ему, иногда создаёт новый узел... Лучше было просто сделать создавалку узла, а ситуацию, когда надо просто поменять value у существующего, разбирать в том месте, где эта функция вызывается.
There was a problem hiding this comment.
Неудачное название переменной, это ведь не новый корень, это просто узел
There was a problem hiding this comment.
| if ((*root)->parent->rightSon == (*root)) | |
| if ((*root)->parent->rightSon == *root) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
"симвовлов", и скобка не закрыта
There was a problem hiding this comment.
Кажется, rightHeight могло бы быть rightSon->height, и leftHeight соответственно leftSon->height, то есть можно хранить только высоту узла. Что сэкономило бы ещё четыре байта.
There was a problem hiding this comment.
Это, наверное, лучше было назвать updateHeight или что-то такое
There was a problem hiding this comment.
| parent->rightSon == (*root) ? attach((*root)->parent, currentRoot, right) : attach((*root)->parent, currentRoot, left); | |
| parent->rightSon == *root ? attach((*root)->parent, currentRoot, right) : attach((*root)->parent, currentRoot, left); |
There was a problem hiding this comment.
| char* newValue = calloc(strlen((currentRoot)->value) + 1, sizeof(char)); | |
| char* newValue = calloc(strlen(currentRoot->value) + 1, sizeof(char)); |
There was a problem hiding this comment.
Вообще, при удалении узла выделять память --- как-то странно. Можно было просто указатели на уже готовые строки перекинуть. Кстати, ещё есть strdup, которая уменьшает боль с копированием строк
There was a problem hiding this comment.
Так Вы проверяете, что у Вас в корне всё хорошо, а во всём остальном дереве?
There was a problem hiding this comment.
Говорит, что узел удалён, даже если такого нету. Там досадный баг в deleteNode
No description provided.