Глейзер Роман by RomanGleyzer · Pull Request #252 · kontur-courses/clean-code

RomanGleyzer · 2025-11-03T08:38:40Z

upd:

Предполагаемый алгоритм обработки текста:

Нам пришел на вход текст: "сер_еди_на тест"
После этого pipeline запускает код, в котором текст превратится в результат
BlockSegmenter - первый этап, он разбивает текст по абзацам. В нашем случае только 1 абзац.
Формируется следующая структура:

Blocks [
  Block {
    RawText: "сер_еди_на тест",
    Inlines: (будут заполнены парсером)
  }
]

Далее парсер обрабатывает сформированные блоки и разбивает текст на вершины (INode) Text, Em, Strong:

Blocks [
  Block {
    RawText: "сер_еди_на тест",
    Inlines: [
      Node("сер", Text),
      Node("еди", Em),
      Node("на тест", Text)
    ]
  }
]

После этого html отрендерит сформированные блоки в:

<p>сер<em>еди</em>на тест</p>

masssha1308 · 2025-11-05T20:19:16Z

@@ -1,14 +1,16 @@



Лучше не включать в МР, т.к. изменения были сгененрированы IDE автоматически и не имеют прямого отношения к МР

masssha1308 · 2025-11-05T20:36:04Z

+
+namespace Markdown;
+
+public class Pipeline(BlockSegmenter blockSegmenter, InlineParser parser, HtmlRenderer htmlRenderer)


На данный момент нет необходимости выносить эту логику в отдельный класс, давай перенесем ее в Md.Render()

masssha1308 · 2025-11-05T21:07:19Z

+    // Предполагается реализация с разделением кода парсинга на отдельные классы, работающие в следующем порядке:
+    // Класс, посимвольно читающий текст
+    // Класс, преобразующий текст в единицы разметки (текст, бэк слеш, одинарное подчеркивание итд)
+    // Класс, отвечающий за обработку экранирования в тексте после преобразования текста в единицы разметки


Экранирование должно обрабатываться до парсинга тегов

masssha1308 · 2025-11-05T21:26:38Z

+        throw new NotImplementedException();
+    }
+
+    // Предполагается реализация с разделением кода парсинга на отдельные классы, работающие в следующем порядке:


Разбиение парсера на 5 классов избыточно, каждый из предложенных классов будет содержать 1-2 метода что усложнит архитектуру без реальной поьзы, классы не будут являться самостоятельными сущностями, а будут лишь шагами алгоритма
Отдельные классы стоило бы выделить в случае переиспользования логики или разных "профилей" парсера (например, с конфигурируемыми настройками), в этом кейсе такой необходимости нет

Возможно для упрощения логики парсера стоит вынести отдельный этап предварительной обработки текста которая подготавливает его к основному парсингу, заменяя "мешающие" элементы на временные маркеры

masssha1308 · 2025-11-05T21:48:00Z

+
+public interface IBlock
+{
+    // Далее планируется добавление двух наследников: HeadingBlock и ParagraphBlock


Нет необходимости в наследниках исходя из текущих требований, лучше добавить поле BlockType. Разница HeadingBlock и ParagraphBlock только в рендеринге, хранятся и обрабатываются данные одинаково. Наследники понадобились бы, например, в случае разных структур данных

masssha1308 · 2025-11-05T21:49:22Z

+
+    public IReadOnlyList<INode> Inlines { get; }
+
+    void SetInlines(IReadOnlyList<INode> inlines);


Какие преимущества перед сеттером? Точно ли нужен этот метод?

Какие преимущества перед сеттером? Точно ли нужен этот метод?

Посидел, подумал и не нашел ни одной причины, по которой бы использование SetInlines было бы лучше использования дефолтного сеттера
Мне кажется под заданные требования будет достаточно просто сеттера. В итоговой реализации поправлю этот момент

masssha1308 · 2025-11-05T22:03:44Z

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

…нтерфейса INode на класс Node

…ытого __

… парсера

…тесты на проверку парсера

masssha1308 · 2025-11-13T14:37:13Z

+
+public class Md
+{
+    public string Render(string text, BlockSegmenter segmenter, InlineParser parser, HtmlRenderer renderer)


Давай передавать зависимости в конструктор, а не в метод. Класс должен скрывать детали реализации. Передавать в метод не стоит, т.к. не получится инжектировать зависимости через di + это нарушает инкасуляцию: любой внешний код вызывающий этот метод будет знать какие зависимости передавать, как их конфигурировать и т.д.

masssha1308 · 2025-11-13T17:16:12Z

+        return position + length < text.Length ? text[position + length] : Space;
+    }
+
+    public static bool IsWordChar(char ch)


Избыточный метод

masssha1308 · 2025-11-13T17:17:06Z

@@ -0,0 +1,24 @@
+Microsoft Visual Studio Solution File, Format Version 12.00


в МР не нужно включать

не нужно тесты выделять в солюшн, проекта достаточно

в МР не нужно включать

Удалил эти файлы

не нужно тесты выделять в солюшн, проекта достаточно

Отказался от .sln. Сейчас в папке проекта нет такого решения

masssha1308 · 2025-11-13T17:19:40Z

+            }
+        }
+        return sb.ToString();
+    }


Метод нечитабельный, нужно разбить/сделать компактнее

masssha1308 · 2025-11-13T17:20:08Z

+        return text?
+            .Replace(PlaceholderUnderscore, Underscore)
+            .Replace(PlaceholderBackslash, Escape)
+            .Replace(PlaceholderHash, '#');


магический символ

masssha1308 · 2025-11-13T17:24:07Z

+        return sb.ToString();
+    }
+
+    private static string RenderInlines(IReadOnlyList<Node> inlines, BlockType context)


Вынесем вспомогательные методы для читабельности? Метод получился большой и есть блоки которые напрашиваются в отдельный метод

masssha1308 · 2025-11-13T17:24:29Z

+                    break;
+
+                case NodeType.Em:
+                    sb.Append("<em>")


магические строки

masssha1308 · 2025-11-13T17:26:45Z

+            switch (node.Type)
+            {
+                case NodeType.Text:
+                    sb.Append(node.Text ?? string.Empty);


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

masssha1308 · 2025-11-13T18:08:46Z

+[TestFixture]
+public class MarkupLanguageProcessorTests
+{
+    private static string RenderHtml(string input)


давай уберем эти вспомогательные методы, они не сокращают кол-во строк в тесте, зато сокращают читабельность: вместо того чтобы открыть тест и посмотреть что там происходит, приходится проваливаться в эти методы и смотреть что там внутри

masssha1308 · 2025-11-13T18:13:23Z

+    }
+
+    [Test]
+    public void ParseAndRender_EmPartOfWord_RendersAtStartMiddleEnd()


Нужно разделить на 3 отдельных теста

masssha1308 · 2025-11-13T18:15:14Z

+    [Test]
+    public void ParseAndRender_EmWholeSentenceFromSpec_ProducesPlainTextHtml()
+    {
+        var input = "Текст, <em>окруженный с двух сторон</em> одинарными символами подчерка, должен помещаться в HTML-тег <em>.";


здесь что проверяется?

здесь что проверяется?

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

здесь что проверяется?

Постарался подобрать более читаемое название. Остановился на варианте "Render_TextWithHtmlTags_KeepsHtmlTags"

masssha1308 · 2025-11-13T18:16:57Z

+    }
+
+    [Test]
+    public void ParseAndRender_OpeningUnderscoreMustBeFollowedByNonSpace_NotOpening()


очень запутанные названия, давай упростим и если требуется добавим описания к тестам
кстати, почему parseandrender когда тестируется метод render?

masssha1308 · 2025-11-13T18:19:10Z

+    }
+
+    [Test]
+    public void ParseAndRender_StrongPartOfWord_RendersAtStartMiddleEnd()


нужно разделить на 3 теста (возможно тесткейсы)

masssha1308 · 2025-11-13T18:20:52Z

+    }
+
+    [Test]
+    public void ParseAndRender_DelimiterFollowedByPunctuation_CommaAfter()


эти 3 логичнее объединить в тест с тесткейсами

masssha1308 · 2025-11-13T18:23:38Z

+            "Но <em>внутри __одинарного__ не</em> работает. Конец\\\\</p>";
+
+        Normalize(RenderHtml(Paragraph)).Should().Be(expected);
+    }


Можно добавить тесты на граничные случаи (например, пустые строки/только пробелы между разделителями/максимальная вложенность/unicode символы и др)

masssha1308 · 2025-11-13T18:27:34Z

+    [Test]
+    public void ParseAndRender_ParagraphFromSpec_AllRulesTogether_ProducesExpectedHtml()
+    {
+        const string Paragraph =


почему константа? почему с большой буквы? отличается от других тестов

masssha1308 · 2025-11-13T18:31:30Z

+    }
+
+    [Test]
+    public void ParseAndRender_ParagraphFromSpec_AllRulesTogether_ProducesExpectedHtml()


антипаттерн, если тест упадет то не понятно почему именно упал и что сломалось, нарушен принцип 1 тест = 1 сценарий

masssha1308 · 2025-11-13T18:42:26Z

+[TestFixture]
+public class PerformanceTests
+{
+    private const string Paragraph =


Один фиксированный паттерн, не отражает реальное использование. Для более реальных тестовых данных можно сделать массив паттернов и рандомно генерировать из них текст

+ можно добавить несколько итераций для усреднения результатов теста

masssha1308 · 2025-11-13T18:43:28Z

+namespace MarkdownTests;
+
+[TestFixture]
+public class PerformanceTests


Можно добавить тесты на конкретные случаи, например, очень большая вложенность

… маг.символ

Предпроверка markdown

1f02013

RomanGleyzer changed the title ~~Предпроверка markdown~~ Глейзер Роман Nov 5, 2025

masssha1308 reviewed Nov 5, 2025

View reviewed changes

RomanGleyzer added 3 commits November 7, 2025 09:22

Подправил текущую реализацию

ffa7354

Реализация сегментера и замена интерфейса IBlock на класс Block

9b17faa

Первичная реализация парсера (нужно будет протестировать) с заменой и…

39c6c27

…нтерфейса INode на класс Node

RomanGleyzer force-pushed the master branch 4 times, most recently from 29a056e to 39c6c27 Compare November 8, 2025 07:37

RomanGleyzer added 6 commits November 8, 2025 13:00

Тесты для парсера

bd7fc44

Правки тестов

6588773

Поправил проверку на оставшиеся открытые выделения

51148eb

Правки тестов и парсера

94823a7

Поправил баг c неправильным сбросом буфера при закрытии _ внутри откр…

d63c22a

…ытого __

Исправил баг с закрытием em, убрал ненужные и добавил новые тесты для…

e36bf9a

… парсера

RomanGleyzer force-pushed the master branch 2 times, most recently from d0d2003 to 0b0c12d Compare November 10, 2025 14:48

Добавил html обработчик и переписал тесты

1ae1901

RomanGleyzer force-pushed the master branch from 0b0c12d to 1ae1901 Compare November 10, 2025 15:22

RomanGleyzer added 2 commits November 11, 2025 21:56

Доработал парсер, сделал упор на улучшение читаемости с соблюдением SRP

7b48e1d

Доработал парсер, добавил тест на производительность и добавил новые …

1b44b9f

…тесты на проверку парсера

RomanGleyzer force-pushed the master branch from 0a3b3de to 1b44b9f Compare November 12, 2025 15:29

masssha1308 reviewed Nov 13, 2025

View reviewed changes

RomanGleyzer added 2 commits November 15, 2025 11:37

Рефакторинг Md для использования di и упрощения BlockSegmenter

02adb0b

Рефакторинг HtmlRenderer. Добавление методов и констант

22c4b3a

RomanGleyzer force-pushed the master branch from c808eed to 8e51515 Compare November 15, 2025 12:26

Inlines в Block теперь без сеттера. Рефакторинг Md, CommitText, убрал…

d32c6e4

… маг.символ

RomanGleyzer force-pushed the master branch from 8e51515 to d32c6e4 Compare November 15, 2025 12:59

RomanGleyzer added 3 commits November 16, 2025 14:37

Рефакторинг парсера и обработки экранирования

1644149

Удалил .sln файлы из репозитория

63d8c03

Улучшил тест производительности и добавил тест по вложенности

d19907c

RomanGleyzer force-pushed the master branch from 272af9b to c1e75be Compare November 17, 2025 12:23

Поправил тесты, добавил новые сценарии, упростил названия

f1fe8e8

RomanGleyzer force-pushed the master branch from c1e75be to f1fe8e8 Compare November 17, 2025 12:33

add .sln

cc310eb


		namespace Markdown;

		public class Pipeline(BlockSegmenter blockSegmenter, InlineParser parser, HtmlRenderer htmlRenderer)


		public IReadOnlyList<INode> Inlines { get; }

		void SetInlines(IReadOnlyList<INode> inlines);

		@@ -0,0 +1,24 @@
		Microsoft Visual Studio Solution File, Format Version 12.00

Conversation

RomanGleyzer commented Nov 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

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!

masssha1308 commented Nov 5, 2025

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!

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!

masssha1308 Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

masssha1308 Nov 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

RomanGleyzer commented Nov 3, 2025 •

edited

Loading

masssha1308 Nov 13, 2025 •

edited

Loading

masssha1308 Nov 13, 2025 •

edited

Loading