Conversation
| using TagsCloudContainer.Result; | ||
|
|
||
| namespace TagsCloudContainer.Core.Ranges; | ||
| public abstract class MaybeRange |
There was a problem hiding this comment.
зачем нужен maybeRange? По сути он дублирует логику result (способ выразить либо значение, либо отсутствие)
| public static readonly MaybeRange Empty = new EmptyRange(); | ||
| public static MaybeRange Range(int min, int max) => new ValueRange(min, max); | ||
|
|
||
| public abstract Result<(int Min, int Max)> GetOrYieldBreak(); |
|
|
||
| var freqRangeResult = FrequencyRange.TryGet(tagList).GetOrYieldBreak(); | ||
| if (!freqRangeResult.IsSuccess) | ||
| return Result<IEnumerable<PositionedTag>>.Failure(freqRangeResult.Error ?? Result<IEnumerable<PositionedTag>>.UnknownError); |
There was a problem hiding this comment.
ошибку лучше устанавливать в FrequencyRange (даже если она неизвестна)
| return Result<IEnumerable<PositionedTag>>.Failure("Tag list is empty."); | ||
|
|
||
| var freqRangeResult = FrequencyRange.TryGet(tagList).GetOrYieldBreak(); | ||
| if (!freqRangeResult.IsSuccess) |
There was a problem hiding this comment.
Хорошо бы избавиться от ручной проверки isSuccess и пробрасывания ошибок. Как вариант посмотреть в сторону Bind в Result
|
|
||
| var ctxResult = BuildRenderContext(request, positionedTags); | ||
| if (!ctxResult.IsSuccess) | ||
| return Result<Image<Rgba32>>.Failure(ctxResult.Error); |
| private Result<ISet<string>> LoadStopWords() | ||
| { | ||
| if (string.IsNullOrEmpty(stopWordsPath) || !File.Exists(stopWordsPath)) | ||
| Result<ISet<string>>.Failure($"Reading error: {stopWordsPath}"); |
|
|
||
| return Result<ISet<string>>.Success( | ||
| File | ||
| .ReadAllLines(stopWordsPath) |
There was a problem hiding this comment.
Кажется тут тоже может возникнуть исключение
| private Result<ISet<string>> LoadStopWords() | ||
| { | ||
| if (string.IsNullOrEmpty(stopWordsPath) || !File.Exists(stopWordsPath)) | ||
| Result<ISet<string>>.Failure($"Reading error: {stopWordsPath}"); |
| foreach (var s in fontChoiceStrategies) | ||
| dict.Add(s.Key, s); | ||
|
|
||
| dict.Add("", fontChoiceStrategies.First()); |
| public static IReadOnlyCollection<string> Choices => | ||
| Strategies.Select(s => s.Key).ToArray(); | ||
|
|
||
| public static Result<FontFamily> Resolve(string? choice) |
There was a problem hiding this comment.
в каких случаях choice может быть null?
| var font = SystemFonts.Collection.Families | ||
| .FirstOrDefault(f => string.Equals(f.Name, name, StringComparison.OrdinalIgnoreCase)); | ||
|
|
||
| return Result<FontFamily>.Success(font); |
| this.request = request ?? throw new ArgumentNullException(nameof(request)); | ||
|
|
||
| public static Result<GenerationContext> Start(TagCloudGenerationRequest request) | ||
| => Result<GenerationContext>.Success(new(request)); |
There was a problem hiding this comment.
чтобы была красивая цепочка Bind
| if (!savingResult.IsSuccess) | ||
| return Result<GenerationContext>.Failure(savingResult.Error!); | ||
|
|
||
| image.Dispose(); |
There was a problem hiding this comment.
лучше использовать using, т.к. в случае если до этого будет исключение ресурс не освободится
| { | ||
| var ffResult = FontFamilyResolver.Resolve(request.Font); | ||
| if (!ffResult.IsSuccess) | ||
| return Result<IReadOnlyCollection<PositionedTag>>.Failure(ffResult.Error ?? Result<IReadOnlyCollection<PositionedTag>>.UnknownError); |
There was a problem hiding this comment.
если !IsSuccess, то ошибка не должна быть пустой
|
|
||
| IWordsSource source = new DocxWordsSource(); | ||
|
|
||
| var words = source.GetWords(path).Value.ToArray(); |
There was a problem hiding this comment.
Хорошо бы добавить проверку IsSuccess, т.к. в противном случае выпадет исключение и скроет реальную ошибку
| "txt" | ||
| ); | ||
|
|
||
| generator.Generate(request); |
|
|
||
| var result = generator.Generate(request); | ||
|
|
||
| result.IsSuccess.Should().Be(false); |
There was a problem hiding this comment.
Лучше проверить более конретную ошибку, например, сверить текст ошибки
2 participants
No description provided.