Conversation
На его основе буду выполнять следующее задание
| System.Console.WriteLine($"Reading words from {inputFile}..."); | ||
|
|
||
| return textReader.ReadWords(inputFile) | ||
| .Then(words => preprocessor.Process(words)) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Поняла про что ты, сам rider мне подсказал как можно сократить)
| static Result<TagCloudSettings> ValidateOptions(CommandLineOptions o) | ||
| { | ||
| if (o.Width <= 0 || o.Height <= 0) | ||
| return Result.Fail<TagCloudSettings>("Invalid image size. Width and height must be positive"); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Вынесла валидацию в Core, в Program теперь только собираю данные и вызываю валидатор, таким образом и от дублирования избавилась
There was a problem hiding this comment.
Я немного другое имел ввиду. У тебя все еще нет гарантии того, что данные в настройках будут валидны, т.к. валидация все равно лежит отдельно от заполнения и никто не мешает пропустить этот шаг и сразу заполнить модель. Как можно гарантировать, что данные в настройках, если они там уже лежат, валидны?
There was a problem hiding this comment.
Я вроде поняла)
В итоге убрала возможность заполнить модель мимо валидации. TagCloudSettings теперь неизменяемый, конструктор у него приватный, а создать экземпляр можно только через метод Create(), который сразу валидирует и возвращает Result
| if (o.Width <= 0 || o.Height <= 0) | ||
| return Result.Fail<TagCloudSettings>("Invalid image size. Width and height must be positive"); | ||
|
|
||
| if (o.MinFontSize <= 0 || o.MaxFontSize <= 0) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Добавила константы для верхних пределов, поставила сейчас на MaxImageSide=10000 и MaxFontSize=500, чтобы не упираться в проблемы с памятью и рендерингом
| json, | ||
| new JsonSerializerOptions { PropertyNameCaseInsensitive = true }); | ||
| if (settings == null) | ||
| throw new InvalidOperationException("Settings file is empty or invalid."); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Упустила этот момент, не выведется из-за Result.Of. Переделала
| .Then(settings => Result.Ok(OverrideBoringWords(settings, o.BoringWordsFile))); | ||
| } | ||
|
|
||
| static Result<TagCloudSettings> ValidateSettings(TagCloudSettings s) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Тут, по сути, такая-же ситуация: сначала заполняем, а потом валидируем. При валидировать нас никто не обязывает, из-за чего ответственность за данные, которые попали в кор, ложится на внешнюю систему (несмотря на то, что сама логика описана в коре)
There was a problem hiding this comment.
Ага, поправила выше, теперь создания без валидации нет
|
|
||
| static Result<TagCloudSettings> ValidateSettings(TagCloudSettings s) | ||
| { | ||
| if (s.Width <= 0 || s.Height <= 0) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Как раз вынести в core, сделала это, выше указала
|
|
||
| static TagCloudSettings OverrideBoringWords(TagCloudSettings s, string? path) | ||
| { | ||
| if (string.IsNullOrWhiteSpace(path)) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Перенесла проверку существования файла в TagCloudSettingsValidator (Core) и вызываю валидатор в методе LoadSettings , чтобы проверить путь
There was a problem hiding this comment.
Не увидел валидации самого BoringWordsPath, при этом тут тоже хочу подчеркнуть, что валидация и заполнение лежат отдельно, из-за чего гарантий вновь нет(
| } | ||
| } | ||
|
|
||
| internal static class SkRectExtensions |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Расширениями вообще на практике особо никогда не пользовалась, тут больше погрузилась в эту тему и изначально (еще в той домашке) решила, что этот блок может переиспользоваться в других алгоритмах. Но сейчас уже понимаю, что тут лучше сделать просто приватный метод, потому что нигде кроме этого класса он не используется
|
|
||
| public Result<IReadOnlyList<string>> ReadWords(string filePath) | ||
| { | ||
| if (!File.Exists(filePath)) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Гарантии нет, File.Exists не проверяет на доступность или валидность.
Сейчас чтение обернула в Result.Of без собственного текста, а через RefineError добавляю путь, за счет этого пользователь видит конкретную причину
| Result<IReadOnlyList<string>> ReadWords(string filePath); | ||
| } | ||
|
|
||
| public interface IFileTextReader : ITextReader |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
У меня есть файловые ридеры - конкретные классы, которые умеют читать один формат (TextFileReader, DocxTextReader), они реализуют IFileTextReader.
А еще есть агрегирующий ридер (MultiFormatTextReader), он выбирает подходящий файловый ридер и делегирует ему работу.
Надобность в IFileTextReader появилась еще в прошлой домашке, цель - чтобы в регистрации в DI не получалось цикла.
В DI регистрация выглядит так:
TextFileReader -> IFileTextReader
DocxTextReader -> IFileTextReader
MultiFormatTextReader -> ITextReader
Далее ConsoleClient просит ITextReader и получает MultiFormatTextReader, а MultiFormatTextReader просит список IFileTextReader, и получает конкретные файловые ридеры.
Если бы MultiFormatTextReader просил IEnumerable, в этот список попал бы он сам, и мы бы получили цикл.
| var info = new SKImageInfo(options.Width, options.Height); | ||
| using var surface = SKSurface.Create(info); | ||
| if (surface == null) | ||
| throw new InvalidOperationException("Failed to create Skia surface"); |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Да, поняла, тут аналогично исправила
TagsCloudContainer.Core/Result.cs
Outdated
|
|
||
| public struct Result<T> | ||
| { | ||
| public Result(string error, T value = default(T)) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
TagsCloudContainer.Core/Result.cs
Outdated
|
|
||
| public static class Result | ||
| { | ||
| public static Result<T> AsResult<T>(this T value) |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Решила убрать AsResult, везде использую Then(Func<TInput,TOutput>)
There was a problem hiding this comment.
Хотел подсветить, что некоторые Then тоже не используется и с ними стоит поступить аналогично)
@Luvr681