Бабинцев Григорий #244
Бабинцев Григорий #244
Conversation
| using Markdown.Parser; | ||
| using FluentAssertions; | ||
|
|
||
| namespace Markdown.Tests |
There was a problem hiding this comment.
Этот файл не подключен к проекту. Скорее всего ты нажал удалить в райдере. Но проекты (csproj) лишь исключаются из решения (sln), а файлы физически остаются. Но это была правильная идея для расположения тестов. Тогда здесь были бы файлы MdTetsts, HtmlBuilderTests, MarkdownParserTests
|
|
||
| private int shift; | ||
|
|
||
| public HtmlBuilder(Dictionary<Tag, string> htmlTagsMarkupDict) |
There was a problem hiding this comment.
Предлагаю его спрятать внутрь реализации. В конструктор выносятся внешние зависимости (Singleton) и реализации интерфейсов. В данном случае словарь мапинга больше похож на внутрянку билдера. Но, можно передавать файл настроек и из него подтягивать, если необходимо
| foreach (var token in tokens) | ||
| { | ||
| var htmlMarkup = _htmlTagsMarkupDict[token.TagType]; | ||
| var tag = token.TagType; |
There was a problem hiding this comment.
Всё-таки это tagType ну или textType
| var htmlMarkup = _htmlTagsMarkupDict[token.TagType]; | ||
| var tag = token.TagType; | ||
|
|
||
| htmlTags.Add(new HtmlTag(tag, token.StartIndex, false, htmlMarkup)); |
There was a problem hiding this comment.
У нас уже имеется htmlMarkup, точно нужно сохранять tag?
| htmlResultText.Insert(tag.Index + shift, tag.GetMarkup()); | ||
| } | ||
|
|
||
| private int GetMdTagLength(HtmlTag tag) |
There was a problem hiding this comment.
Ой какая страшная логика. Сюда будет сложно добавить обработку нового типа. Может добавим в тэг информацию о длине? Тогда сможем сделать маркдаун независимую реализацию генератора.
| needClosingTags.Clear(); | ||
| offsetTags.Clear(); | ||
|
|
||
| currentIndex = 0; |
There was a problem hiding this comment.
А вот здесь и теряются индексы при переносе строк. Достаточно сохранять общий offset до начала текущей строки (не забывая про \n) и тогда ситуация починится
|
|
||
| public List<Token> ParseMarkdown(string markdownText) | ||
| { | ||
| var lines = markdownText.Split('\n'); |
There was a problem hiding this comment.
Хорошая идея работать с одной и той же строкой, быстро и экономно по памяти. Но здесь концепция нарушается при разбиении на отдельные строки (ещё одна копия входных данных). Думаю, написать на индексах по строке, с проверкой на \n или сразу получить список индексов для всех переносов будет не очень сложно, а выигрыш будет по памяти в два раза
| return fountedTokens; | ||
| } | ||
|
|
||
| private void SearchTokensInLine(string line, List<Token> fountedTokens) |
| @@ -0,0 +1,2 @@ | |||
| // See https://aka.ms/new-console-template for more information | |||
| Console.WriteLine("Hello, World!"); | |||
There was a problem hiding this comment.
У тестов не делают Program файл и в целом Main метод, так как точка входа генерируется при сборке фреймворком для тестов
| @@ -0,0 +1,2 @@ | |||
| // See https://aka.ms/new-console-template for more information | |||
| Console.WriteLine("Hello, World!"); | |||
There was a problem hiding this comment.
В Program обычно пишут какой-нибудь базовый код запуска, чтобы не приходилось думать, а где точка входа. Здесь можно написать в сокращённом виде
var builder = new HtmlBuilder(MarkdownConfig.HtmlTags);
var parser = new MarkdownParser(new TagChecker(), MarkdownConfig.DifferentTags, MarkdownConfig.MdTags);
var md = new Md(parser, builder);
var text = File.ReadAllText("Input.md");
File.WriteAllText("Result.html", md.Render(text));
Или по старинке
namespace Markdown;
public static class Program
{
public static void Main(string[] args)
{
var builder = new HtmlBuilder(MarkdownConfig.HtmlTags);
var parser = new MarkdownParser(new TagChecker(), MarkdownConfig.DifferentTags, MarkdownConfig.MdTags);
var md = new Md(parser, builder);
var text = File.ReadAllText("Input.md");
File.WriteAllText("Result.html", md.Render(text));
}
}
Но тогда компилятор будет ругаться на несколько точек входа, так как тесты генерируют свой Main код. Для этого их и выносят в отдельный проект
| } | ||
| } | ||
|
|
||
| public static IEnumerable<TestCaseData> TestsWithIntersectionTags |
There was a problem hiding this comment.
Не хватает теста на пересечение тегов "H _e __l_ l__ o", "H _e __l_ l__ o"
cs/Markdown/Parser/MarkdownParser.cs
Outdated
| { | ||
| var shift = _mdTags[token.TagType].Length; | ||
| var diffTagType = _differentTagTypes[token.TagType]; | ||
| var anyWhiteSpace = line.Substring(token.StartIndex + 1, token.EndIndex - token.StartIndex - 1).Any(char.IsWhiteSpace); |
There was a problem hiding this comment.
Создаёт копию строки. Лучше написать экстеншн, который проверит наличие пробелов через индекс
| @@ -0,0 +1,53 @@ | |||
| using FluentAssertions; | |||
There was a problem hiding this comment.
Есть рекомендация писать отдельные тесты ещё и на каждый из модулей. То есть отдельно проверять, что правильно работает парсер, и отдельно - что правильно генерируется итоговый текст. Помогает понять, в каком месте именно происходит ошибка. Ну и сильно сокращает написание тестов при добавлении ещё одного типа оформления
No description provided.