Кирилл Зарипов #232
Кирилл Зарипов #232
Conversation
| return tokens; | ||
| } | ||
|
|
||
| private int HandleEscape(string markdown, int index, StringBuilder text) |
There was a problem hiding this comment.
Он удаляет одиночные \, так не должно быть)
| return index + count; | ||
| } | ||
|
|
||
| private int CountUnderscores(string text, int index) |
There was a problem hiding this comment.
Попробую описать, что делает метод: Считает идет 1 или 2 _ подряд.
Название CountUnderscores точно не подходит.
И, кажется, проблема не в названии, а в сути метода. Мне кажется, тут больше подойдет метод, который определяет "что за токен перед нами", в который можно вынести подобную логику.
|
|
||
| private bool IsDelimiter(char value) => value != '_' && (char.IsLetter(value) || char.IsPunctuation(value)); | ||
|
|
||
| private Token CreateTextToken(string content) => new() { Type = TokenType.Text, Content = content }; |
There was a problem hiding this comment.
Такое можно вынести в сам Token. Или даже лучше в его Extension методы
|
|
||
| private Token CreateTextToken(string content) => new() { Type = TokenType.Text, Content = content }; | ||
|
|
||
| private void CreateTokenPairs(List<Token> tokens, TokenType type, string markdown) |
There was a problem hiding this comment.
Еще перед прочтением кода - метод капец какой длинный, его тяжело читать. Нужно держать слишком много логики в голове
There was a problem hiding this comment.
После прочтения - да, слишком много логики.
И switch pattern в комбинации с IsOpening, IsClosing добивает это все.
Опять изменение состояний объекта.
Изначально подход сомнительный и не оптимальный, потому что после приходится бегать и удалять "неправильные" пары. Такое обычно делается при помощи КДА
There was a problem hiding this comment.
ДКА интрересная штука, но сейчас уже что есть то есть
cs/Markdown/Program.cs
Outdated
| var converter = new HtmlConverter(); | ||
| var markdown = new Md(tokenizer, converter); | ||
|
|
||
| const string input = """ |
There was a problem hiding this comment.
Советы категории Б: можешь вынести эту строчку в константу, чтобы потом, при добавлении новых примеров, можно было легко между ними переключаться :)
const string firstExample = "This _is_ a __sample__ markdown _file_.\n";
const string secondExample = "#This is another __sample__ markdown _file_";
var input = firstExample;
There was a problem hiding this comment.
А еще давай сюда буду накидывать, в каких случаях все ломается:
___Some Text___- должно стать<strong><em>Some</strong></em>__Some_- должно стать_<em>Some</em>Some_Another_Text- не должно измениться123\456- не должно измениться
| return result; | ||
| } | ||
|
|
||
| private bool IsValidDelimiter(string markdown, int index, int length, out bool isOpening, out bool isClosing) |
There was a problem hiding this comment.
Столько входящих параметров тяжело переварить человеческому мозгу... Точнее, ответить на вопрос: "Зачем для проверки на валидность нужно 5 аргументов"
А еще out параметры имеются...
| return index + count; | ||
| } | ||
|
|
||
| text.Append(new string('_', count)); |
There was a problem hiding this comment.
new string('_', count) дублируется.
Да и я бы сказал, что завязка на _ слишком сильная. Это ведь может быть любой другой символ)
Я, например, обычно использую **Text** вместо __Text__. А так md поддерживает и тот, и другой.
Я к тому, что будет тяжело масшабировать
| if (token.Pair == null) | ||
| continue; | ||
|
|
||
| switch (token.Type) |
There was a problem hiding this comment.
Виднеется закономерность: как только в коде появляется switch/case он становится менее читаемым.
Как раз для таких вещей придумали полиморфизм)
| { TokenType.Strong, "strong" }, | ||
| }; | ||
|
|
||
| public string Convert(List<Token> tokens) |
There was a problem hiding this comment.
Давай принимать обобщенные типы)
В этом случае вообще IEnumerable достаточно
| html.Append(token.Type switch | ||
| { | ||
| TokenType.Text => token.Content, | ||
| TokenType.Italic or TokenType.Strong when token.Pair != null => |
There was a problem hiding this comment.
Нарушение SRP. Класс должен нагенерить html по входным данным, а он еще что-то проверяет
@w1jtoo