Result pattern by d2em0n · Pull Request #1 · d2em0n/di-updated

d2em0n · 2025-01-19T09:41:53Z

No description provided.

GlazProject

В целом направление верное. Осталось доработать оставшиеся классы.

TagGenerator
PictureMaker
TextProcessor
IWordFilter

[PictureMaker] _tags = tagGenerator.GenerateTags(textProcessor.WordFrequencies()); лучше перенести в метод, а не в конструктор. Это связано с тем, что в идеале создание класса не должно зависеть от аргументов исполнения, а сам класс должен быть переиспользуемым, если это не data-class. То есть один раз созданный PictureMaker должен без пересоздания уметь читать подряд разные файлы. Для этого ITextProcessor должен в методе принимать параметры чтения, чтобы их использовал TextProvider. Это к вопросу о том, что должно быть в конструкторе, а что в методе. В качестве одной из аналогий можно привести грузовик. Без колёс, кузова и кабины он не будет грузоваиком. Поэтому они передаются в конструкторе. А вот наполнение кузова (песок, щебень, скала) - это уже объекты, требуемые для осуществления конкретной операции - перевозки. Поэтому груз будет передаваться как аргумент метода перевозки

GlazProject · 2025-01-19T10:01:04Z

            var container = DependencyInjection.BuildContainer(config);
            using var scope = container.BeginLifetimeScope();
            scope.Resolve<PictureMaker>().DrawPicture();
            Console.WriteLine($"результат сохранен в {config.PicturePath}");


Нужно обернуть внутрь Then. Иначе мы попытаемся собрать контейнер с неверным Config

GlazProject · 2025-01-19T10:03:38Z

-            throw new FileNotFoundException();
+            return new Result<string>($"File not found: {_filePath}");
        using var stream = new FileStream(_filePath, FileMode.Open, FileAccess.Read);
        var document = new HWPFDocument(stream);


Здесь тоже может быть ошибка чтения документа. Стрим оборачивать в result непросто, так как его нужно диспоузить. Поэтому для простоты можно считать, что он откроется всегда
return Result.Of(() => new HWPFDocument(stream)).Then(document=> document.GetRange())

GlazProject · 2025-01-19T10:12:42Z

-        return File.ReadAllText(_filePath);
+        return !File.Exists(_filePath) 
+            ? Result.Fail<string>($"File {_filePath} does not exist") 
+            : File.ReadAllText(_filePath);


ReadAllText может вернуть целый ворох исключений. Обернём его в Result.Of(), чтобы они не сломали наше приложение?

GlazProject · 2025-01-19T10:14:15Z

+
+                return Result.Fail<Rectangle>("Вышли за границы рисунка");
            }
            throw new ArgumentException("Not Enough Points Generated");


Если возвращается result, то не должно быть throw

GlazProject · 2025-01-19T10:17:21Z

        {
            var rectangle = layout.PutNextRectangle(tag.Frame);
-            DrawTag(image, rectangle, tag);
+            DrawTag(image, rectangle.GetValueOrThrow(), tag);


Думаю, лучше использовать Result как возвращаемое значение, чем бросать исключение просто так. Не стоит смешивать всё в одно. Либо исключения, либо Result. В этом задании нужно именно Result использовать, чтобы почувствовать, как с ним работать. Пусть, возможно, без него и легче читается конкретно этот пример.

GlazProject · 2025-01-19T10:18:19Z

+        }
+    }
+
+    public static Result<TOutput> Then<TInput, TOutput>(


Этот метод и ниже можно вынести в Extensions, чтобы не забивать всем подряд Result

GlazProject · 2025-01-19T10:19:54Z

    public Dictionary<Word, int> WordFrequencies()
    {
-        var words = parser.GetWordsFromString(provider.ReadFile());
+        var words = parser.GetWordsFromString(provider.ReadFile().GetValueOrThrow());


Давай тоже придерживаться Result подхода. Тогда должно быть

return provider.ReadFile().Then(text => parser.GetGetWordsFromString(text)).Then(words => filters.Aggregate(...))

GlazProject · 2025-01-21T11:39:38Z

            using var scope = container.BeginLifetimeScope();
-            scope.Resolve<PictureMaker>().DrawPicture();
-            Console.WriteLine($"результат сохранен в {config.PicturePath}");
+            scope.Resolve<PictureMaker>().DrawPicture().OnFail(error => Console.WriteLine($"Ошибка обработки: {error}"))


OnFail лучше с новой строки )

GlazProject · 2025-01-21T11:40:38Z

        using var image = new Bitmap(layout.Size.Width, layout.Size.Height);
-        foreach (var tag in _tags)
+        var wordsDictionary = _textProcessor.WordFrequencies();
+        if (!wordsDictionary.IsSuccess)


А как же Then?

GlazProject · 2025-01-21T11:41:15Z

        {
            var rectangle = layout.PutNextRectangle(tag.Frame);
-            DrawTag(image, rectangle.GetValueOrThrow(), tag);
+            if (!rectangle.IsSuccess)


Здесь без явной проверки к сожалению не обойтись. Всё правильно

GlazProject · 2025-01-21T11:42:02Z

-        var document = new HWPFDocument(stream);
-        var range = document.GetRange();
-        return range.Text;
+        return Result.Of(() => new HWPFDocument(stream)).Then(document=> document.GetRange().Text);


С переносами будет чуть читаемее

GlazProject · 2025-01-23T05:11:33Z

-            DrawTag(image, rectangle, tag);
+
+        return _textProcessor.WordFrequencies()
+            .Then(wordsDict =>


Огромный кусок кода в Then. В отладке это место будет очень больно проходить

GlazProject · 2025-01-23T05:11:52Z

+            .Then(wordsDict =>
+            {
+                var tags = _tagGenerator.GenerateTags(wordsDict);
+                if (!tags.IsSuccess) return new Result<None>(tags.Error);


Для предотвращения такого кода был сделан Then

GlazProject · 2025-01-23T05:30:35Z

+                image.Save(_fileName);
+                return Result.Ok();
+            })
+            .OnFail(error => Result.Fail<None>(error));


Кается этот метод выглядит также как код ниже

var a = 15; var b = a; return b;

GlazProject · 2025-01-23T05:31:51Z

+                foreach (var tag in tags.Value)
+                {
+                    var rectangle = layout.PutNextRectangle(tag.Frame);
+                    if (!rectangle.IsSuccess)
+                        return Result.Fail<None>(rectangle.Error);;
+                    DrawTag(image, rectangle.Value, tag);
+                }


Уже не первый раз вызывается ForEach. Поэтому лучше вынести в экстеншн и не писать никакие

if (!rectangle.IsSuccess) return Result.Fail<None>(rectangle.Error);

GlazProject · 2025-01-23T05:33:04Z

+            if (string.IsNullOrWhiteSpace(input))
+                return Result.Fail<IEnumerable<Word>>("Input cannot be empty");
+            return Result.Ok(_regex.Matches(input)
                .Cast<Match>()


Matches уже возвращает IEnumerable

GlazProject · 2025-01-23T05:34:05Z

+                .Select(kvp =>
+                {
+                    var colorResult = _colorProvider.GetColor();
+                    if (!colorResult.IsSuccess) 
+                        return Result.Fail<Tag>(colorResult.Error);
+
+                    var fontResult = SetFont(_defaultFont, kvp.Value);
+                    if (!fontResult.IsSuccess)
+                        return Result.Fail<Tag>(fontResult.Error);
+
+                    var frameSizeResult = SetFrameSize(kvp.Key, fontResult.Value, 1, _graphics);
+                    if (!frameSizeResult.IsSuccess)
+                        return Result.Fail<Tag>(frameSizeResult.Error);
+
+                    return Result.Ok(new Tag(kvp.Key, fontResult.Value, colorResult.Value,
+                        frameSizeResult.Value));
+                });


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

GlazProject · 2025-01-23T05:36:43Z

+            return Result.Ok(tagsResult.Select(t => t.Value))
+                .OnFail(error => Result.Fail<Tag>(error));


t.Value всегда вернёт значение. Либо null, либо настоящее. В итоге в OnFail никогда не попадёт исполнение кода. Если вызывать GetValueOrThrow, то на пустом месте получим исключение, которое затормозит процесс.

Так как IEnumerable ленивый, то мы исключение получим только в рантайме при итерации по нему. А Result будет всегда успешный

d2em0n added 5 commits January 16, 2025 20:15

add Result.cs

dc8b502

some exceptions

99391eb

result pattern for comfiguration

b02e7bc

result pattern to other exceptions

f705e0d

OnFail except checking

e9192f1

GlazProject reviewed Jan 19, 2025

View reviewed changes

d2em0n added 4 commits January 19, 2025 18:49

senseis notes are taken into account

79b05fc

old tests have to pass!

110e46f

ITagGenerator goes back to roots)

23fa57d

result class core with extensions

6f1ae2e

GlazProject reviewed Jan 21, 2025

View reviewed changes

d2em0n added 6 commits January 21, 2025 21:20

aestetics

e12f33c

result to colorprovider

c6874fe

result for SetFrameSize

03427f6

taggenerator is almost perfect )

827c66f

taggenerator now better than almost perfect )

9ae55ec

result for IStringParser

c3d34a1

GlazProject reviewed Jan 23, 2025

View reviewed changes

d2em0n added 3 commits January 23, 2025 12:05

refactor PictureMaker with ForEach extension for result

ae3ab55

refactor RegexParser

364001f

remake TagGenerator

011abf2

		return Result.Ok(tagsResult.Select(t => t.Value))
		.OnFail(error => Result.Fail<Tag>(error));

Conversation

d2em0n commented Jan 19, 2025

Uh oh!

GlazProject 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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants