Conversation
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Алгоритмически вроде всё ок, но надо больше тестов, и есть пара технических замечаний.
| Assert.IsTrue(tree.IsConsist(i.ToString())); | ||
| } | ||
| } | ||
|
|
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| @@ -0,0 +1,30 @@ | |||
| using System; | |||
|
|
|||
| namespace hw3B_tree | |||
There was a problem hiding this comment.
Пространства имён именуются с заглавной (и проекты стоит сразу с заглавной называть)
| /// </summary> | ||
| public class BTree | ||
| { | ||
| public int MinimumDegreeOfTree { get; set; } |
There was a problem hiding this comment.
Ой. А что будет, если у уже наполненного значениями дерева, сделать set этому полю? :)
| MinimumDegreeOfTree = minimumDegreeOfTree; | ||
| } | ||
|
|
||
| private Node FindNood(string key) |
| /// 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.
Exists или что-то такое было бы более по-английски
| } | ||
| } | ||
| return false; | ||
| } |
There was a problem hiding this comment.
Эти три метода имеют одинаковую структуру. Найти узел, покопаться в его сыновьях, сделать что-то и вернуть что-то. Можно было ещё больше унифицировать, тем более теперь, когда вы знаете лямбда-функции. Но раз это третья домашка, со времён, когда лямбда-функций ещё не было, можно не править :)
| { | ||
| return false; | ||
| } | ||
| return String.Compare(key, keyInNode) < 0 ? true : false; |
There was a problem hiding this comment.
| return String.Compare(key, keyInNode) < 0 ? true : false; | |
| return String.Compare(key, keyInNode) < 0; |
|
|
||
| private Node root; | ||
|
|
||
| private Node runner; |
There was a problem hiding this comment.
Это лучше везде сделать локальной переменной и передавать как параметр в рекурсивные функции. Потому что поля --- это в принципе информация, которая должна сохраняться между вызовами паблик-методов. Иначе возникает масса вопросов относительно инвариантов и взаимозависимости между методами по данным (вот один метод попользовался runner-ом, а второй забыл его выставить в начальное значение).
| bool check; | ||
| if (index == runner.CountKeys) | ||
| { | ||
| check = true; | ||
| } | ||
| else | ||
| { | ||
| check = false; | ||
| } |
There was a problem hiding this comment.
| 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) |
There was a problem hiding this comment.
| for ( int i = index + 1; i < node.CountKeys; ++i) | |
| for (int i = index + 1; i < node.CountKeys; ++i) |
yurii-litvinov
left a comment
yurii-litvinov
left a comment
There was a problem hiding this comment.
Стало гораздо лучше, но недостаточно лучше --- всё ещё есть какие-то мелкие огрехи, а если вспомнить, что из-за мелких огрехов в программе можно потерять десятки миллионов долларов (см., например, 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.
| 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.
Лишняя пустая строчка
| tree = new BTree(MinimumDegreeOfTree); | ||
| for (int i = 1; i < 19; ++i) | ||
| { | ||
| tree.Insert(i.ToString(), i.ToString()); |
There was a problem hiding this comment.
Можно было через интерполяцию строк, 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.
А чего не AreEqual? Тут и ниже
| { | ||
| if (minimumDegreeOfTree < 2) | ||
| { | ||
| throw new ArgumentException("Минимальная степень дерева выбрана неправильна!"); |
There was a problem hiding this comment.
| 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) |
There was a problem hiding this comment.
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> |
There was a problem hiding this comment.
| /// <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> |
| if (root.Leaf) | ||
| { | ||
| root = null; | ||
| } | ||
| else | ||
| { | ||
| root = root.Sons[0]; | ||
| } |
There was a problem hiding this comment.
root = root.Leaf ? null : root.Sons[0];
| } | ||
| else | ||
| { | ||
| cursor = node;//////// |
There was a problem hiding this comment.
//////// не нужен, тем более что, кажется, cursor тут и так равен node
3 participants
No description provided.