Конина Анастасия #181
Конина Анастасия #181
Conversation
TagCloud/CloudDrawer.cs
Outdated
| return Result.Of(() => | ||
| { | ||
| var bitmap = new Bitmap(pictureSize.Width, pictureSize.Height); | ||
| var gr = Graphics.FromImage(bitmap); |
There was a problem hiding this comment.
Graphic - IDisposable объект - надо освобождать после использования.
TagCloud/CloudDrawer.cs
Outdated
| public Result<Bitmap> DrawCloud(Result<IEnumerable<WordForCloud>> wordsForCloud) | ||
| { | ||
| if (!wordsForCloud.IsSuccess) | ||
| return wordsForCloud.Then(x => new Bitmap(0, 0)); |
There was a problem hiding this comment.
new Bitmap(0, 0) упадет с Parameter not valid. При использовании таких значений, лучше руководствоваться здравывм смыслом - скорее всего они не приведут к валидному результату.
| if (tagCloudSettings.Value == null) | ||
| return; |
There was a problem hiding this comment.
Предполагается, что IsSuccess должно говорить о том, что настройки не null.
| this Result<TInput> input, | ||
| string error) | ||
| { | ||
| return input.ReplaceError(err => input.IsSuccess ? error : input.Error); |
There was a problem hiding this comment.
Судя по названию метода имелось в виду заменять текст ошибки, если он пустой?
TagCloud/WordsForCloudGenerator.cs
Outdated
| .OrderBy(x => x.Value) | ||
| .Reverse() |
TagCloud/WordsForCloudGenerator.cs
Outdated
| colorGenerator.GetNextColor(), | ||
| x.Key, | ||
| x.Value, | ||
| maxFrequency: frequency.FirstOrDefault().Value))); |
There was a problem hiding this comment.
Тут, вместо вычисления (выборки) максимума каждый раз можно вычислить один раз, на еще одном промежуточном шаге Then.
| return !boringWords.IsSuccess | ||
| ? Result.Fail<List<string>>(boringWords.Error) | ||
| : wordsResult |
There was a problem hiding this comment.
Может тут можно просто boringWords.Then?
There was a problem hiding this comment.
А фактически, метод NormalizeWords не нуждается в Results на входе и выходе, тут в самом методе нечему падать.
TagCloud/CloudDrawer.cs
Outdated
| this.pictureSize = pictureSize; | ||
| } | ||
|
|
||
| public Result<Bitmap> DrawCloud(Result<IEnumerable<WordForCloud>> wordsForCloud) |
There was a problem hiding this comment.
Тут можно ведь принять на вход просто IEnumerable<WordForCloud>?
Зачем проверять IsSuccess внутри метода, если можно "проверить" снаружи и просто не вызывать данный метод?
TagCloud/TagCloudCreator.cs
Outdated
| var words = wordsReader.Get(inputFile); | ||
| var normalizedWords = wordsNormalizer.NormalizeWords(words, | ||
| wordsReader.Get(boringWordsFile) | ||
| .Then(boringWords => boringWords.ToHashSet())); | ||
| var wordsForCloud = wordsForCloudGenerator.Generate(normalizedWords); | ||
| return cloudDrawer.DrawCloud(wordsForCloud); |
No description provided.