Шевырин Никита #229
Шевырин Никита #229
Conversation
|
|
||
| public enum MarkdownTokenType | ||
| { | ||
| NoConversion, |
There was a problem hiding this comment.
Это зачем нужно и где использоваться будет?
| { | ||
| NoConversion, | ||
| ToItalic, | ||
| ToBold, |
There was a problem hiding this comment.
По смыслу если это именно тип, то To из названия можно убрать
Markdown/Markdown/Md.cs
Outdated
| { | ||
| var markdownSpan = markdown.AsSpan(); | ||
| var context = new StringBuilder(); | ||
| var stepCount = markdownSpan.Length / _sliceSize; |
There was a problem hiding this comment.
Не очень понял, зачем эта разбивка на куски вообще нужна. Не сломается ли чего если какой-то токен сразу в двух слайсах окажется?
Markdown/Markdown/Md.cs
Outdated
|
|
||
| public class Md : IMd | ||
| { | ||
| private readonly ITokenizer[] _tokenizers = |
There was a problem hiding this comment.
В других домашках ты вроде по-другому приватные поля называл, без _ . В этом проекте всегда так поля называть будем?
Markdown/Markdown/Md.cs
Outdated
| var sliceStart = step * _sliceSize; | ||
| var sliceSize = Math.Min(_sliceSize, markdownSpan.Length - sliceStart); | ||
| var stepSpan = markdownSpan.Slice(sliceStart, sliceSize); | ||
| foreach (var tokenizer in _tokenizers) |
There was a problem hiding this comment.
Я предлагаю разделить логику на несколько классов:
- Сначала преобразуем текст в набор токенов
- Преобразуем набор токенов в другой текст
А то тяжело как-то следить за тем что вообще происходит
| [Description("Тест на производительность")] | ||
| public void Render_PerformanceTest() | ||
| { | ||
| var fullStr = ArrangePerformanceTest("_Hello_ world_12. Hel_lo world_", 20000); |
There was a problem hiding this comment.
Круто что сделал тест на производительность, есть парочка моментов, которые можно было бы улучшить.
- Хорошо бы было проверять разные входные данные с разными тегами, экранированием, вложенностью и т.п. Кажется можно задавать параметры где-то перед тестами, напиример в тесткейсе
- Хотелось бы на каждом запуске теста проверять скорость рендера не 1 раз, а несколько, и высчитывать среднее значение. Т.к. какие-то запуски могут работать чуть медленнее н-р из-за сборки мусора, или работы каких-то внешних библиотек
- Хорошо бы отделить юнит-тесты от нагрузочных, и вынести это в отдельный проект
- Общую логику для теста тоже можно вынести в отдельный класс (запуск какого-то метода, измерение его времени и логирование результата). Вдруг будем не только рендер тестировать.
| InsideWord = insideWord; | ||
| } | ||
|
|
||
| public ReadOnlyMemory<char> Text { get; set; } |
There was a problem hiding this comment.
В каких-то классах у тебя поля классов до конструктора прописаны, в каких-то после. Давай одного стиля придерживаться
| using Markdown.Tokenizer; | ||
|
|
||
| var tokenAliases = new Dictionary<string, MdTokenType>(); | ||
| tokenAliases.Add("_", MdTokenType.Italic); |
There was a problem hiding this comment.
Может вынесем инициализацию этих объектов в какой-то отдельный класс? Его тогда и в тестах переиспользовать можно будет
|
|
||
| namespace Markdown.SyntaxRules; | ||
|
|
||
| public class NestingRule : ISyntaxRule<MdTokenType> |
There was a problem hiding this comment.
Не кажется что у NestingRule, TokensInDifferentWordsRule и NumberRule много общего кода, и отличается только одна проверка. Можно ли как-то общий функционал вынести?
| input.Slice(plainTextStart, str!.Length - plainTextStart)); | ||
| } | ||
|
|
||
| private bool TryMatchTokenAliases( |
There was a problem hiding this comment.
Можно вместо 3 out переменных какой-нибудь класс создать, и его возвращать, чуть читаемее будет
| return true; | ||
| } | ||
|
|
||
| if (mathcedOpeningToken) |
There was a problem hiding this comment.
Можно вынести в отдельные методы обработки типов токенов
|
|
||
| private bool IsWordDelimiter(char c) | ||
| { | ||
| return c is ' ' or '\t' or '\n' or '\r' or ',' or '.' |
There was a problem hiding this comment.
Это как-будто константой должно быть или где-то извне передаваться
| int i = 0; | ||
| foreach (var ch in pattern) | ||
| { | ||
| if (index + i >= input.Length || ch != input[index + i]) |
There was a problem hiding this comment.
Мы тут за пределы строки не можем в теории выйти?
No description provided.