Русинов Матвей #261

nbdevncrs · 2025-11-04T22:17:38Z

SquirrelLeonid · 2025-11-05T09:26:08Z

Давай тут отвечу для истории. Сначала непосредственно по твоим вопросам.

Я создал TokenPosition, который определяет позицию токена в изначальном тексте, но это нужно исключительно для тестирования, чтобы какое-то конкретное место выкидывать для наглядности, то есть функциональности не несет. в таком случае его лучше оставить, убрать, или без разницы?

Отрезать лишнее всегда легче, чем пришить необходимое. Если этот участок кода не мешает препятствует выполнению задачи, то ничего такого в том, чтобы он пока был. Но сдается мне, что этот класс в том или ином виде тебе может пригодиться.

У меня получилось реально много разных классов для нодов, большая часть наследуется даже от абстрактных классов BlockNode и InlineNode, которые относятся к INode, то есть разделяются на блочные и строчные. для меня это выглядит неприятно, что есть так много классов, но при этом сама логика выглядит вполне нормальной, и я не знаю как это улучшить/поправить, или может в целом попусту парюсь. в общем, был бы рад, если бы ты смог на ревью с этим моментом помочь или расписать чуть подробнее.

Не вижу здесь проблемы. У тебя каждый класс отвечает за определенный тэг в разметке - можно сказать это естественно, что их такое количество.

SquirrelLeonid · 2025-11-05T14:04:09Z

Tokenizer

CharStream

Использование Stream в имени как бы наталкивает на мысль о том, что объект должен освобождать какие-то ресурсы (как минимум реализовывать IDisposable). В действительности нам здесь не нужно ничего освобождать. Я бы предложил поменять имя на CharCursor или TextCursor.
В целом, наверное такой вопрос по этому классу - он действительно необходим? С текущим скелетом кажется, что можно обойтись простым обходом for по строке. Я не предлагаю прям сейчас его удалять - возможно он обрастет какими-то деталями при реализации.

TokenType

Какие значения будут представлены в перечислении TokenType?

TokenPosition

Текст можно рассматривать с разных сторон в зависимости от текущего контекста и задачи. К примеру:

Если мы работаем с логами работы приложения, то нам удобно рассматривать текст как набор строк, где каждая строка представляет собой отдельную запись в логе (дата, уровень сообщения, кто отправил, текст ошибки и т.п.).
Или мы можем рассматривать текст как последовательность символов. Ведь новая строка не что иное как пара символов \r\n (если говорить о Windows)

В рамках этой задачи нам может быть удобнее рассматривать текст именно во втором варианте, ведь каждый отдельный символ для нас может иметь значение

Parsing

Декомпозиция выглядит неплохо. Вижу, что учтены не только отдельные теги, но и область их действия (тег может / не может входить в другой тег). Один момент:

Я бы предложил подумать над вариантами имен для BlockNode и InlineNode. Вроде бы понятно, что они под собой подразумевают, но, кажется, смысл немного ускользает.

В целом по декомпозиции

Из текущего скелета видно, как оно может прийти к итоговому решению. Над декомпозицией хорошо поработал.
Для меня пока остаются открытыми два момента:

Вопрос о перечислении TokenType (смотри выше)
Вопрос о том, как обрабатывать пересечения между тегами (когда один может исключать другой, например). Хотя в целом могу представить, что логика может быть размещена где-то на уровне Parser

Предпроверку засчитываю. В итоговом решении обязательно покрой код тестами (тут действительно отличная практика для TDD)

…code, token class and tokenize method

…ucture, finished ParserCursor.cs and Parser.cs with all logic

…layers to Md.cs

SquirrelLeonid

Оставил множество замечаний по стилистике кода, именованию переменных и т.д.
Ключевых момента три:

Постараться упростить UnderscoreHandler
Аналогично для LinkHandler
Сделать рефакторинг в классе HtmlRenderer. Я бы сказал, что у этого наивысший приоритет.

В остальном решение весьма неплохое на мой взгляд, но шлифануть код надо.

SquirrelLeonid · 2025-11-12T13:19:57Z

cs/Markdown/Md.cs

+using Parsing;
+using Rendering;
+
+public class Md


При текущей реализации класс в целом может быть статичным. В тестах можно опустить методы SetUp по его созданию.
Хотя можно оставить некоторый прогрев, перед использованием

SquirrelLeonid · 2025-11-12T13:21:44Z

cs/Markdown/Parsing/EscapeHandler.cs

+
+public static class EscapeHandler
+{
+    public static void HandleEscape(List<InlineTypeNode> children, ParserCursor cursor, IList<Token> tokens)


Есть неиспользуемый параметр

SquirrelLeonid · 2025-11-12T13:22:30Z

cs/Markdown/Parsing/LinkHandler.cs

+    public static void HandleLink(List<InlineTypeNode> children, ParserCursor cursor, IList<Token> tokens)
+    {
+        var link = TryParseLink(cursor, tokens);
+        if (link != null) children.Add(link);


Не ошибка, но я предпочитаю размещать if и строку кода в отдельных строках. Тут на твое усмотрение.
В других таких ситуациях аналогично.

SquirrelLeonid · 2025-11-12T13:23:28Z

cs/Markdown/Parsing/LinkHandler.cs

+        }
+    }
+
+    private static LinkNode? TryParseLink(ParserCursor cursor, IList<Token> tokens)


Если не ошибаюсь, то знак вопроса в LinkNode? можно вообще опустить. Это и так ссылочный тип.

Попробуй в .csproj убрать строку с добавлением nullable.

SquirrelLeonid · 2025-11-12T13:24:43Z

cs/Markdown/Parsing/LinkHandler.cs

+
+    private static LinkNode? TryParseLink(ParserCursor cursor, IList<Token> tokens)
+    {
+        var rightBracket = FindTokenBeforeEol(tokens, cursor.Index + 1, TokenType.RightBracket);


Если переменная хранит индекс, то в имени это тоже стоит отразить. Т.е. будет лучше rightBracketIndex, например. Другие переменные тоже посмотри на такое проявление

SquirrelLeonid · 2025-11-12T13:56:30Z

cs/Markdown/Tokenizing/Tokenizer.cs

+    {
+        ArgumentNullException.ThrowIfNull(text);
+
+        var cursor = new CharCursor(text);


тоже можно уточнить имя до charCursor. Упрощаем чтение кода, когда уйдем далеко от этой строчки

SquirrelLeonid · 2025-11-12T13:56:39Z

cs/Markdown/Tokenizing/Tokenizer.cs

+
+        while (!cursor.End)
+        {
+            var c = cursor.Current;


лучше избегать таких простых названий. Хоть и очевидно, что это текущий символ, но будет хорошо так его и назвать

SquirrelLeonid · 2025-11-12T13:57:52Z

cs/Markdown/Tokenizing/Tokenizer.cs

+        return tokens;
+    }
+
+    private static void ReadText(List<Token> tokens, CharCursor cursor)


Лучше избегать Side-эффектов, в частности наполнения листа. Будет лучше сменить сигнатуру метода так, чтобы он возвращал полученный токен. Ну и соответственно название поменять согласованно с новой сигнатурой.

SquirrelLeonid · 2025-11-12T13:58:29Z

cs/Markdown/Tokenizing/Tokenizer.cs

+            tokens.Add(new Token(TokenType.Text, sb.ToString()));
+    }
+
+    private static bool IsTextChar(char c)


Могу предложить написать альтернативный подход, записав значения один раз в словарь или HashSet.
Тогда в месте вызова этого метода можно будет перейти на вызов Contains

SquirrelLeonid · 2025-11-12T13:59:28Z

cs/Markdown/Tokenizing/TokenType.cs

@@ -0,0 +1,17 @@
+namespace Markdown.Tokenizing;
+
+public enum TokenType


Тут подсвечу два момента.

Полезной практикой является явная нумерация значений в перечислении.
Это полезно несколькими вещами:
а) Явный контроль над числовым представлением того или иного значения (enum по сути - набор целочисленных констант)
б) Существенно сокращает вероятность ошибки в случаях, когда ты пишешь маппинг из сущностей одного уровня (транспортный), в сущности другого уровня (бизнес-логика). Напиши мне отдельно, если нужен пример

Всегда полезно в качестве первого значения резервировать Unknown, со значением 0. Это способствует обратной совместимости

…nder method to INode, so nodes now implementing it, depending on render way they need

…uge logic between new functions, it also works with a cursor now, so no -1, 0, and 1 semantic in UnderscoreHandler.cs needed now. some fields now renamed

SquirrelLeonid · 2025-11-13T08:17:59Z

В свежих правках увидел то, что хотел. HtmlRenderer стал попроще и теперь логика рендера конкретного блока - это ответственность самого блока. Если подумать, то идею можно развить и дальше с точки зрения добавления рендеров в другие форматы разметки. Но считаю это за рамками этой задачи.

UnderscoreHandler тоже стало намного приятнее читать. На методы хорошо разбил. Семантика с -1 0 1 в каком то виде осталась, но теперь это вполне конкретная операция по поиску индекса, что гуд.

Задачу на максимальный балл засчитываю, но посмотри и другие комментарии.

nbdevncrs added 5 commits November 5, 2025 03:10

added markdown project with Md class

6c41c3e

added tokens structure and logic to markdown processor

fde6e5b

added all nodes structure that parser needs to parse tokens

1b63fd3

added parser logic with parser cursor for cleaner parser code

227ea8e

added a renderer structure that will make html from tree (parsed tokens)

99321a8

nbdevncrs added 5 commits November 12, 2025 00:56

added tests for test-driven development for most layers

3b41898

created tokenizing layer: added token types, charstream for readable …

2f68375

…code, token class and tokenize method

added all nodes types for parser and added logic that they need

9d54ea8

completed parsing layer: added helper-layers for more comfortable str…

9a6654e

…ucture, finished ParserCursor.cs and Parser.cs with all logic

markdown processor finished: added html-renderer layer and added all …

bd54531

…layers to Md.cs

SquirrelLeonid reviewed Nov 12, 2025

View reviewed changes

nbdevncrs added 2 commits November 12, 2025 23:26

HtmlRenderer.cs refactor: made rendering polymorhpic by adding HtmlRe…

ee65e1c

…nder method to INode, so nodes now implementing it, depending on render way they need

UnderscoreHandler.cs refactor: TryToClose edited with splitting its h…

6029dfe

…uge logic between new functions, it also works with a cursor now, so no -1, 0, and 1 semantic in UnderscoreHandler.cs needed now. some fields now renamed

		@@ -0,0 +1,17 @@
		namespace Markdown.Tokenizing;

		public enum TokenType

Русинов Матвей #261

Are you sure you want to change the base?

Русинов Матвей #261

Uh oh!

Conversation

nbdevncrs commented Nov 4, 2025

Uh oh!

SquirrelLeonid commented Nov 5, 2025

Uh oh!

SquirrelLeonid commented Nov 5, 2025

Tokenizer

CharStream

TokenType

TokenPosition

Parsing

В целом по декомпозиции

Uh oh!

SquirrelLeonid left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SquirrelLeonid commented Nov 13, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants