-
Notifications
You must be signed in to change notification settings - Fork 0
Hw3 b tree #5
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?
Hw3 b tree #5
Conversation
yurii-litvinov
left a comment
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.
Алгоритмически вроде всё ок, но надо больше тестов, и есть пара технических замечаний.
| Assert.IsTrue(tree.IsConsist(i.ToString())); | ||
| } | ||
| } | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
hw3B-tree/hw3B-tree/Program.cs
Outdated
| @@ -0,0 +1,30 @@ | |||
| using System; | |||
|
|
|||
| namespace hw3B_tree | |||
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.
Пространства имён именуются с заглавной (и проекты стоит сразу с заглавной называть)
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| /// </summary> | ||
| public class BTree | ||
| { | ||
| public int MinimumDegreeOfTree { get; set; } |
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.
Ой. А что будет, если у уже наполненного значениями дерева, сделать set этому полю? :)
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| MinimumDegreeOfTree = minimumDegreeOfTree; | ||
| } | ||
|
|
||
| private Node FindNood(string key) |
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.
FindNode?
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| /// 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) |
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.
Exists или что-то такое было бы более по-английски
| } | ||
| } | ||
| return 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.
Эти три метода имеют одинаковую структуру. Найти узел, покопаться в его сыновьях, сделать что-то и вернуть что-то. Можно было ещё больше унифицировать, тем более теперь, когда вы знаете лямбда-функции. Но раз это третья домашка, со времён, когда лямбда-функций ещё не было, можно не править :)
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| { | ||
| return false; | ||
| } | ||
| return String.Compare(key, keyInNode) < 0 ? true : 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.
| return String.Compare(key, keyInNode) < 0 ? true : false; | |
| return String.Compare(key, keyInNode) < 0; |
hw3B-tree/hw3B-tree/BTree.cs
Outdated
|
|
||
| private Node root; | ||
|
|
||
| private Node runner; |
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.
Это лучше везде сделать локальной переменной и передавать как параметр в рекурсивные функции. Потому что поля --- это в принципе информация, которая должна сохраняться между вызовами паблик-методов. Иначе возникает масса вопросов относительно инвариантов и взаимозависимости между методами по данным (вот один метод попользовался runner-ом, а второй забыл его выставить в начальное значение).
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| bool check; | ||
| if (index == runner.CountKeys) | ||
| { | ||
| check = true; | ||
| } | ||
| else | ||
| { | ||
| check = 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.
| bool check; | |
| if (index == runner.CountKeys) | |
| { | |
| check = true; | |
| } | |
| else | |
| { | |
| check = false; | |
| } | |
| bool check = index == runner.CountKeys; |
hw3B-tree/hw3B-tree/BTree.cs
Outdated
|
|
||
| private void DeleteFromLeaf(int index, Node node) | ||
| { | ||
| for ( int i = index + 1; i < node.CountKeys; ++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.
| for ( int i = index + 1; i < node.CountKeys; ++i) | |
| for (int i = index + 1; i < node.CountKeys; ++i) |
yurii-litvinov
left a comment
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.
Стало гораздо лучше, но недостаточно лучше --- всё ещё есть какие-то мелкие огрехи, а если вспомнить, что из-за мелких огрехов в программе можно потерять десятки миллионов долларов (см., например, 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) |
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.
| public BTree Setup(int MinimumDegreeOfTree) | |
| public BTree Setup(int minimumDegreeOfTree) |
| for (int i = 1; i < 19; ++i) | ||
| { | ||
| tree.Insert(i.ToString(), i.ToString()); | ||
|
|
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.
Лишняя пустая строчка
| tree = new BTree(MinimumDegreeOfTree); | ||
| for (int i = 1; i < 19; ++i) | ||
| { | ||
| tree.Insert(i.ToString(), i.ToString()); |
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.
Можно было через интерполяцию строк, tree.Insert($"{i}", $"{i}");, но дело вкуса
| var tree = Setup(2); | ||
| for (int i = 1; i <= 18; ++i) | ||
| { | ||
| Assert.IsTrue(tree.GetValue(i.ToString()) == i.ToString()); |
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.
А чего не AreEqual? Тут и ниже
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| { | ||
| if (minimumDegreeOfTree < 2) | ||
| { | ||
| throw new ArgumentException("Минимальная степень дерева выбрана неправильна!"); |
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.
| throw new ArgumentException("Минимальная степень дерева выбрана неправильна!"); | |
| throw new ArgumentException("Минимальная степень дерева выбрана неправильно!"); |
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| /// 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) |
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.
IsExists --- это переводится примерно как "есть существует". Просто Exists грамматически правильнее и вполне общепринято
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| /// <summary> | ||
| /// get value by key | ||
| /// </summary> | ||
| /// <returns>return value, if we find key and return null,if we don't find key</returns> |
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.
| /// <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> |
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| if (root.Leaf) | ||
| { | ||
| root = null; | ||
| } | ||
| else | ||
| { | ||
| root = root.Sons[0]; | ||
| } |
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.
root = root.Leaf ? null : root.Sons[0];
hw3B-tree/hw3B-tree/BTree.cs
Outdated
| } | ||
| else | ||
| { | ||
| cursor = node;//////// |
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.
//////// не нужен, тем более что, кажется, cursor тут и так равен node
yurii-litvinov
left a comment
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.
👍
No description provided.