Skip to content

Comments

avlTree#36

Open
Andrw-404 wants to merge 6 commits intomainfrom
hw9-AVLTree
Open

avlTree#36
Andrw-404 wants to merge 6 commits intomainfrom
hw9-AVLTree

Conversation

@Andrw-404
Copy link
Owner

No description provided.


#include <stdbool.h>

typedef struct Node Node;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Я как пользователь не хочу никаких узлов, я хочу словарь, отображающий строку в строку. Стоило бы хотя бы затайпдефить Node на Dictionary и использовать Dictionary везде в функциях интерфейса модуля, чтобы пользователя не пугать

}

int getHeight(Node* node) {
return node ? node->height : -111;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Почему -111?


newNode->value = malloc(strlen(value) + 1);
if (newNode->value == NULL) {
return NULL;

This comment was marked as resolved.


newNode->key = malloc(strlen(key) + 1);
if (newNode->key == NULL) {
return NULL;

This comment was marked as resolved.

return createNode(value, key);
}

int num = atoi(key);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Никто не обещал, что ключ конвертируется в число

if (node == NULL) {
return NULL;
}
if (strcmp(node->key, key) == 0) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Вот тут же сравнение правильное, почему-то в add другое (кстати, если add использует один порядок, а search — другой, двоичное дерево поиска вообще работать не будет).

Comment on lines 175 to 177
free((char*)root->key);
free((char*)root->value);
free(root);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделали бы отдельную функцию удаления узла

Comment on lines 187 to 188
strcpy((char*)root->key, temp->key);
strcpy((char*)root->value, temp->value);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Тут тоже не факт, что это возможно

Comment on lines 13 to 23
// small left rotate
Node* rotateLeft(Node* node);

// small right rotate
Node* rotateRight(Node* node);

// big left rotate
Node* bigLeftRotate(Node* node);

// bit right rotate
Node* bigRightRotate(Node* node);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Надо удалить из интерфейса модуля функции, которые к нему не относятся

printf("\n\n");
switch (choice)
{
case(1):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
case(1):
case 1:


newNode->value = strdup(value);
if (newNode->value == NULL) {
free((char*)newNode->key);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А это плохая идея, он ещё не инициализирован и не забит нулями (потому что malloc, а не calloc), упадёт сразу

Comment on lines 103 to 105
free((char*)node->value);
node->value = strdup(value);
if (node->value == NULL) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Так мы старый value уже удалим, а новый не создадим. Лучше

char* temp = strdup(value);
if (temp == NULL) {
    return NULL;
}
free((char*)node->value);
node->value = strdup(value);

Но это не сильно поможет, потому что факт возврата NULL в строках 97 и 100 не проверяется, так что там всё равно дерево пострадает и это даже не сообщат вызывающему.

root->value = strdup(tmp->value);
root->left = tmp->left;
root->right = tmp->right;
free(tmp);

This comment was marked as resolved.

Comment on lines 35 to 38
int getHeight(Dictionary* node);

// function to update node height
void updateHeight(Dictionary* node); No newline at end of file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Думаю, что это пользователю не нужно

Comment on lines 85 to 86
fgets(key, 100, stdin);
key[strcspn(key, "\n")] = '\0';

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Сделали бы функцию чтения строки. Всего две строчки но десяток раз в программе

typedef struct Dictionary Dictionary;

// returns the balance of the tree
int getBalance(Dictionary* node);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Это тоже пользователю не нужно, но могло бы быть полезно в тестах, чтобы доказать, что дерево реально АВЛ. Если бы был такой тест, и если бы эта функция проверяла бы баланс по всему дереву, а не только в корне (ну, точнее, это должна быть другая функция, специально для тестов, а getBalance можно убрать в .c)

feat: isAVL function and readString function
test: add isAVLTest
Copy link

@yurii-litvinov yurii-litvinov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ну почти. Но тут фиксы тривиальны, так что думаю, что можно зачесть

return NULL;
}
free((char*)node->value);
node->value = strdup(value);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
node->value = strdup(value);
node->value = tmp;

Провал :) Не воспринимайте мои подсказки столь буквально

if (tmp < 0) {
node->left = add(node->left, key, value);
if (node->left == NULL) {
return NULL;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

А оно так последовательно забудет указатели на всё левое поддерево, выходя из рекурсии. Надо было примерно как с strdup, запомнить во временную переменную, проверить, и если получилось плохо, дерево не менять.

#include <string.h>
#include <stdbool.h>

bool isAVLTest() {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
bool isAVLTest() {
bool isAVLTest(void) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants