Сахабутдинов Ильфир #184
Сахабутдинов Ильфир #184
Conversation
| private static void HandleParseError(IEnumerable<CommandLineError> errs) | ||
| { | ||
| Console.WriteLine("Ошибка при настройке."); | ||
| Environment.Exit(1); |
There was a problem hiding this comment.
Не очень хорошо что у приложения несколько точек выхода. Я бы назвал их внезапными. Будет неплохо, если прокинешь результат до main и вернешь его на выходтам.
There was a problem hiding this comment.
Поправил, теперь вывожу сообщения ошибок в консоль в классе TagsCloudContainerUi
| internal class WeigherWordSizer(IImageSettingsProvider imageSettingsProvider) : IWeigherWordSizer | ||
| { | ||
| public Result<IReadOnlyCollection<ViewWord>> CalculateWordSizes(IReadOnlyDictionary<string, int> wordFrequencies, | ||
| int minSize = 8, int maxSize = 24) |
There was a problem hiding this comment.
Тут бы проверку параметров добавить. Больше к тому, что если minSize будет больше maxSize, то new Font упадет с исключением, которое в этом классе не ловится.
There was a problem hiding this comment.
Если они оба отрицательные, то, наверное, конструктору шрифта тоже не понравится?
There was a problem hiding this comment.
Согласен, не учел. Добавил проверку и тесты на эти случаи
| private const int DistanceLayersDifference = 1; | ||
| private const double BetweenAngleDifference = Math.PI / 36; | ||
|
|
||
| public Result<None> SetCenterPoint(Point center) |
There was a problem hiding this comment.
Вроде бы, нет смысла возвращать Result, если тут не может возникнуть ошибка. Сложно представить ошибку при установке центра.
|
|
||
| public Result<Point> GetNextRectangleLocation(Size rectangleSize) | ||
| { | ||
| if (rectangleSize.Width == 0 || rectangleSize.Height == 0) |
There was a problem hiding this comment.
Теоретически может быть и меньше нуля.
|
|
||
| private static Bitmap CreateImageMap(ImageSettings imageSettings) | ||
| { | ||
| var bitmap = new Bitmap(imageSettings.Size.Width, imageSettings.Size.Height); |
There was a problem hiding this comment.
Bitmap - IDisposable, где-то бы его освободить...
There was a problem hiding this comment.
Добавил освобождение в TagsCloudContainerUi
| IWordFormatter<WordDetails> wordFormatter) | ||
| : ITextPreprocessor | ||
| { | ||
| public Result<IReadOnlyDictionary<string, int>> GetWordFrequencies(string text, WordSettings settings) |
There was a problem hiding this comment.
Опять же, предположу, что во внутренностях этого метода ошибки возникнуть не может.
There was a problem hiding this comment.
Переделал работу с анализатором. Теперь может упасть ошибка
| return new Error($"Файл по указанному пути '{filePath}' не существует."); | ||
| } | ||
|
|
||
| using var fileStream = new FileStream(filePath, FileMode.Open, FileAccess.ReadWrite); |
There was a problem hiding this comment.
Теоретически, тут может упасть с исключением. По многим поводам, например: файл мог исчезнуть в момент после проверки выше и это строкой, могут быть различные ошибки прав доступа, файл может быть залочен для чтения и т.п.
|
|
||
| internal sealed class WordReader : IWordReader | ||
| { | ||
| public Result<IReadOnlyCollection<string>> ReadWords(string text) |
There was a problem hiding this comment.
В методе может возникнуть ошибка?
| var speechParts = validSpeechParts.ToArray(); | ||
| if (speechParts.Length == 0) | ||
| { | ||
| return new Error("Должен быть указан хотя бы одна доступная часть речи"); |
| "jpg" => ImageFormat.Jpeg, | ||
| "jpeg" => ImageFormat.Jpeg, | ||
| "bmp" => ImageFormat.Bmp, | ||
| _ => throw new ArgumentException($"Unsupported image format: {ImageFormatString}") |
| private static Result<None> HandleParseError(IEnumerable<CommandLineError> errs) | ||
| { | ||
| var errors = errs.Select(err => err.Tag.ToString()).ToList(); | ||
| return new Error($"При чтении опций произошла ошибка. Теги ошибок: {errors}"); |
There was a problem hiding this comment.
При чтении опций произошла ошибка. Теги ошибок: System.Collections.Generic.List`1[System.String]
| .OnFail(error => | ||
| { | ||
| Console.WriteLine(error.Message); | ||
| Environment.Exit(1); |
There was a problem hiding this comment.
А что мешает вернуть ошибку ещё выше и вывести в консоль там?
Если говорить точнее, мне не нравится как ты выходишь из приложения. У этого метода есть свои особенности, и это не является нормальным способом выходить.
There was a problem hiding this comment.
Перенес вывод сообщения выше. Спасибо за ссылку, учту это для себя
| internal class WeigherWordSizer(IImageSettingsProvider imageSettingsProvider) : IWeigherWordSizer | ||
| { | ||
| public Result<IReadOnlyCollection<ViewWord>> CalculateWordSizes(IReadOnlyDictionary<string, int> wordFrequencies, | ||
| int minSize = 8, int maxSize = 24) |
There was a problem hiding this comment.
Если они оба отрицательные, то, наверное, конструктору шрифта тоже не понравится?
…кулятора размеров слов
2 participants
@lev_shisterov