Skip to content

Conversation

@MinyazevR
Copy link
Owner

No description provided.

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.

Местоположение этих проектов как-то против принятых в остальном репозитории соглашений.

{
printf("����������, ������� ����\n");
int key = 0;
const int firstScanfResilt = scanf("%d", &key);

Choose a reason for hiding this comment

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

firstScanfResult, тут и ниже (в нескольких местах)

}
printf("����������, ������� ������, ������� ������ ��������\n");
char string[100] = {'\0'};
const int secondScanfResult = scanf("%s", string);

Choose a reason for hiding this comment

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

scanf_s лучше, у нас ведь входной буфер ограничен 99 символами, о чём мы пользователя даже не предупреждаем, кстати

}
}

void zig(Node* x)

Choose a reason for hiding this comment

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

В этой задаче дерево не обязательно должно быть сбалансированным, можно было сэкономить кучу сложности. Хотя раз мы всё равно на паре балансировку написали, то почему нет

Tree/Tree/Tree.c Outdated
char* copyValue = calloc(100, sizeof(char));
if (copyValue == NULL)
{
return NULL;

Choose a reason for hiding this comment

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

Это не очень хорошо работает с tree = addNode(tree, key, string); из main-а. Потому что тогда в main надо писать что-то в духе

Tree *temp = addNode(tree, key, string);
if (temp == NULL) 
{
    deleteTree(tree);
    return -1;
}
tree = temp;

Что слишком утомительно. Лучше поступать как scanf — сообщать об ошибке, но само дерево не трогать.

Tree/Tree/Tree.c Outdated
Comment on lines 269 to 273
if (root == NULL)
{
return false;
}
return true;

Choose a reason for hiding this comment

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

Так писать нельзя :)

Tree/Tree/Tree.c Outdated
}


Node* addNode(Node* root, int key, char* value)

Choose a reason for hiding this comment

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

Думаю, что const char *value, мы ведь не собираемся его менять тут. Это позволит компилятору не ругаться на строковые литералы при вызове функции.

Tree/Tree/Tree.c Outdated
if (root->parent->rightSon == root)
{
root->parent->rightSon = NULL;
root->parent = NULL;

Choose a reason for hiding this comment

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

Это делается в любом случае, поэтому можно вынести за if. Но вообще, мы сейчас root удалим, о значениях его полей беспокоиться незачем. Всё равно что хорошенько прибраться в доме перед его сносом — можно, но зачем?

Tree/Tree/Tree.c Outdated
{
if (root->parent == NULL)
{
Node* j = root->leftSon;

Choose a reason for hiding this comment

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

Вообще, кажется, стайлгайд запрещает имена переменных короче четырёх символов

Tree/Tree/Tree.c Outdated
Comment on lines 355 to 363
root->leftSon->parent = root->parent;
if (root->parent->leftSon == root)
{
root->parent->leftSon = root->leftSon;
}
else if (root->parent->rightSon == root)
{
root->parent->rightSon = root->leftSon;
}

Choose a reason for hiding this comment

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

Опять-таки, что-то такое делает attach, а если нет, то стоит это вынести в функцию. Потому что очень уж сложно получилось

break;
}
tree = deleteNode(tree, key);
printf("�������� �������\n");

Choose a reason for hiding this comment

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

Даже если удаление не удалось :)

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.

Остались только "косметические" замечания, так что зачтена.

Comment on lines +29 to +30

Choose a reason for hiding this comment

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

Не совсем, и непонятно ни по названию, ни по комментарию, что она делает. Тем более что у дерева может быть много инвариантов (у Вас, например, что node->parent == NULL || node->parent->leftSon == node || node->parent->rightSon == node — это тоже инвариант). Интересно, что инвариант "является деревом поиска" функция как раз не проверяет — она не смотрит, что больший ключ справа :) Скорее, это проверка, что узел с secondKey является сыном узла firstKey, это и стоило отразить хотя бы в комментарии (и может даже назвать функцию isChild).

Comment on lines +172 to +175

Choose a reason for hiding this comment

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

Suggested change
}
Node* addNode(Node* root, int key, const char* value, Error* error)
}
Node* addNode(Node* root, int key, const char* value, Error* error)

Choose a reason for hiding this comment

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

Я б её назвал currentNode. Хотя это, безусловно, корень текущего поддерева, но кажется, что мы используем её просто для спуска по дереву.

Comment on lines +274 to +275

Choose a reason for hiding this comment

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

Можно было просто

Suggested change
Node* searchResult = search(root, key);
return searchResult != NULL;
return search(root, key) != NULL;

Хотя Ваш вариант немного удобнее для отладки.

Choose a reason for hiding this comment

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

Suggested change
if ((*root)->parent->rightSon == (*root))
if ((*root)->parent->rightSon == *root)

Choose a reason for hiding this comment

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

Suggested change
return;

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