Skip to content

New bor#5

Open
Palezehvat wants to merge 9 commits intomasterfrom
NewBor
Open

New bor#5
Palezehvat wants to merge 9 commits intomasterfrom
NewBor

Conversation

@Palezehvat
Copy link
Owner

Bor, 2 попытка

Choose a reason for hiding this comment

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

Назвали бы заодно его как-то по-человечески. На английском он Trie, а транслитом нельзя писать (если что, C# позволяет имена русскими буквами, но это адово неудобно).

Choose a reason for hiding this comment

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

Suggested change
private BorElement root = new BorElement();
private BorElement root = new();

Comment on lines 13 to 16

Choose a reason for hiding this comment

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

Кажется, можно доказать, что root никогда не может быть null, и nullability-анализ это может проверить. Стоит считать этот факт инвариантом и убрать все проверки.

Choose a reason for hiding this comment

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

Нет, null нельзя добавлять в словарь, пусть оно с ArgumentNullException в этом случае падает. Пустую строку можно, но это другое.

Choose a reason for hiding this comment

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

Почему бы не хранить в walker.Next символы, а не непонятные int-ы?

Choose a reason for hiding this comment

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

Suggested change
if (isDeleted == true)
if (isDeleted)

Не надо сравниваться с булевыми константами, == true — это тождественная функция в булевой алгебре

Comment on lines 114 to 118

Choose a reason for hiding this comment

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

Это лучше через тернарный оператор переписать

Choose a reason for hiding this comment

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

Suggested change
public int HowManyStartsWithPrefix(String prefix)
public int HowManyStartsWithPrefix(string prefix)

Это одно и то же, но лучше использовать ключевое слово, обозначающее тип, а не имя типа.

Comment on lines 160 to 162

Choose a reason for hiding this comment

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

По-английски это звучит как "Моя твоя башка дубина бить". "Размер" — "Size", "Словарь" — "Dictionary", значит "РазмерСловаря" — "SizeDictionary"? :)

Choose a reason for hiding this comment

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

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

Comments