Skip to content

Conversation

@MikePuzanov
Copy link
Owner

No description provided.

Copy link
Collaborator

@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.

Алгоритмически вроде всё ок, но надо больше тестов, и есть пара технических замечаний.

Assert.IsTrue(tree.IsConsist(i.ToString()));
}
}

This comment was marked as resolved.

@@ -0,0 +1,30 @@
using System;

namespace hw3B_tree
Copy link
Collaborator

Choose a reason for hiding this comment

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

Пространства имён именуются с заглавной (и проекты стоит сразу с заглавной называть)

/// </summary>
public class BTree
{
public int MinimumDegreeOfTree { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ой. А что будет, если у уже наполненного значениями дерева, сделать set этому полю? :)

MinimumDegreeOfTree = minimumDegreeOfTree;
}

private Node FindNood(string key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

FindNode?

/// checking for the presence of a key in a tree
/// </summary>
/// <returns>true - if key is in tree, false - if key isn't in tree</returns>
public bool IsConsist(string key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Exists или что-то такое было бы более по-английски

}
}
return false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Эти три метода имеют одинаковую структуру. Найти узел, покопаться в его сыновьях, сделать что-то и вернуть что-то. Можно было ещё больше унифицировать, тем более теперь, когда вы знаете лямбда-функции. Но раз это третья домашка, со времён, когда лямбда-функций ещё не было, можно не править :)

{
return false;
}
return String.Compare(key, keyInNode) < 0 ? true : false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
return String.Compare(key, keyInNode) < 0 ? true : false;
return String.Compare(key, keyInNode) < 0;


private Node root;

private Node runner;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Это лучше везде сделать локальной переменной и передавать как параметр в рекурсивные функции. Потому что поля --- это в принципе информация, которая должна сохраняться между вызовами паблик-методов. Иначе возникает масса вопросов относительно инвариантов и взаимозависимости между методами по данным (вот один метод попользовался runner-ом, а второй забыл его выставить в начальное значение).

Comment on lines 297 to 305
bool check;
if (index == runner.CountKeys)
{
check = true;
}
else
{
check = false;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
bool check;
if (index == runner.CountKeys)
{
check = true;
}
else
{
check = false;
}
bool check = index == runner.CountKeys;


private void DeleteFromLeaf(int index, Node node)
{
for ( int i = index + 1; i < node.CountKeys; ++i)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
for ( int i = index + 1; i < node.CountKeys; ++i)
for (int i = index + 1; i < node.CountKeys; ++i)

Copy link
Collaborator

@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.

Стало гораздо лучше, но недостаточно лучше --- всё ещё есть какие-то мелкие огрехи, а если вспомнить, что из-за мелких огрехов в программе можно потерять десятки миллионов долларов (см., например, https://ru.wikipedia.org/wiki/%D0%9C%D0%B0%D1%80%D0%B8%D0%BD%D0%B5%D1%80-1), надо доисправлять.

{
private BTree tree;

public BTree Setup(int MinimumDegreeOfTree)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
public BTree Setup(int MinimumDegreeOfTree)
public BTree Setup(int minimumDegreeOfTree)

for (int i = 1; i < 19; ++i)
{
tree.Insert(i.ToString(), i.ToString());

Copy link
Collaborator

Choose a reason for hiding this comment

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

Лишняя пустая строчка

tree = new BTree(MinimumDegreeOfTree);
for (int i = 1; i < 19; ++i)
{
tree.Insert(i.ToString(), i.ToString());
Copy link
Collaborator

Choose a reason for hiding this comment

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

Можно было через интерполяцию строк, tree.Insert($"{i}", $"{i}");, но дело вкуса

var tree = Setup(2);
for (int i = 1; i <= 18; ++i)
{
Assert.IsTrue(tree.GetValue(i.ToString()) == i.ToString());
Copy link
Collaborator

Choose a reason for hiding this comment

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

А чего не AreEqual? Тут и ниже

{
if (minimumDegreeOfTree < 2)
{
throw new ArgumentException("Минимальная степень дерева выбрана неправильна!");
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
throw new ArgumentException("Минимальная степень дерева выбрана неправильна!");
throw new ArgumentException("Минимальная степень дерева выбрана неправильно!");

/// checking for the presence of a key in a tree
/// </summary>
/// <returns>true - if key is in tree, false - if key isn't in tree</returns>
public bool IsExists(string key)
Copy link
Collaborator

Choose a reason for hiding this comment

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

IsExists --- это переводится примерно как "есть существует". Просто Exists грамматически правильнее и вполне общепринято

/// <summary>
/// get value by key
/// </summary>
/// <returns>return value, if we find key and return null,if we don't find key</returns>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// <returns>return value, if we find key and return null,if we don't find key</returns>
/// <returns>return value, if we find key and return null, if we don't find key</returns>

Comment on lines 254 to 261
if (root.Leaf)
{
root = null;
}
else
{
root = root.Sons[0];
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

root = root.Leaf ? null : root.Sons[0];

}
else
{
cursor = node;////////
Copy link
Collaborator

Choose a reason for hiding this comment

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

//////// не нужен, тем более что, кажется, cursor тут и так равен node

Copy link
Collaborator

@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.

👍

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.

3 participants