Trie #3
Trie #3
Conversation
Trie/Program.cs
Outdated
There was a problem hiding this comment.
Эээ, тут что-то странное
Пользователь, запустивший программу, ничего не поймёт.
Вообще, можно делать исключительно library сборки (без консольного интерфейса), если в задаче иного не сказано
Trie/Trie.csproj
Outdated
There was a problem hiding this comment.
CSC : warning SA0001: XML comment analysis is disabled due to project configuration
Trie/Trie.cs
Outdated
| /// </summary> | ||
| public Vertex() | ||
| { | ||
| this.childs = []; |
There was a problem hiding this comment.
warning SA1010: Opening square brackets should not be preceded by a space.
Trie/Trie.cs
Outdated
| @@ -0,0 +1,193 @@ | |||
| /// <summary> | |||
There was a problem hiding this comment.
warning SA1514: Element documentation header should be preceded by blank line
Trie/Program.cs
Outdated
There was a problem hiding this comment.
CSC : warning SA1516: Elements should be separated by blank line
Пишут, что такое бывает с C#9 top-level statements. Можно поставить бета-версию, в которой это работает адекватно:
dotnet add package StyleCop.Analyzers --version 1.2.0-beta.556
TrieTests/TrieTest.cs
Outdated
There was a problem hiding this comment.
Вообще, было бы неплохо вынести папки Trie и TrieTests в отдельную папку (например, Trie/Trie - код бора, Trie/TrieTests - тесты). Когда станет много проектов в корне репозитория в этом всём станет немного тяжело ориентироваться.
Trie/Trie.cs
Outdated
| /// <summary> | ||
| /// Gets the trie size. | ||
| /// </summary> | ||
| public int Size => this.root.CountOfTerminalsFarther; | ||
|
|
||
| /// <summary> | ||
| /// Adds element to trie. | ||
| /// </summary> | ||
| /// <param name="element">Element to add to the trie.</param> | ||
| /// <returns>True if success, else false.</returns> | ||
| public bool Add(string element) | ||
| { | ||
| return this.root.BuildNext(element); | ||
| } |
There was a problem hiding this comment.
Когда каждый метод реализован как проброс вызова на корень, начинает казаться, что можно было бы иметь один класс Trie с реализацией как у Vertex 🤔
Trie/Trie.cs
Outdated
|
|
||
| private class Vertex | ||
| { | ||
| private readonly Dictionary<char, Vertex> childs; |
There was a problem hiding this comment.
| private readonly Dictionary<char, Vertex> childs; | |
| private readonly Dictionary<char, Vertex> children; |
Trie/Trie.cs
Outdated
| /// <summary> | ||
| /// Builds next vertex. | ||
| /// </summary> |
There was a problem hiding this comment.
Если бы я не знал, что эта функция - реализация Add, из названия и описания вообще не понял бы, что происходит
| public int Size => this.CountOfTerminalsFarther; | ||
|
|
||
| private int CountOfTerminalsFarther { get; set; } |
There was a problem hiding this comment.
Не уверен, почему бы не объединить их в одно поле, но если уж делаем private ..., то геттеры и сеттеры писать не надо.
Без геттеров/сеттеров - это будет честное поле класса. С геттерами/сеттерами это будет поле класса, спрятанное под функциями гет и сет.
There was a problem hiding this comment.
Стоило бы добавить хоть один тест на Size, раз уж это часть публичного API, в остальном норм
There was a problem hiding this comment.
Вообще, я имел ввиду не убирать целиком анализатор, а отключать конкретные варнинги, которые себя криво ведут 🤔
No description provided.