Сазонов Александр #27
Conversation
|
|
||
| namespace TagCloud.Logic.CloudLayouts; | ||
|
|
||
| public class SimpleCloudLayout : ICloudLayout |
There was a problem hiding this comment.
Предлагаю вместо терминологии Simple использовать Default или Standard или ещё как-нибудь, а то как будто обесцениваешь свою работу, что написал самый простой "simple" код))
Во всей кодовой базе предлагаю обновить
| public IReadOnlyCollection<IWordTag> GetTags(IEnumerable<string> words, IWordHandler wordHandler, | ||
| ISizeCalculator sizeCalculator) |
There was a problem hiding this comment.
- А почему эти зависимости не через конструктор получил? Они ведь зареганы в конструкторе
- Не очень понимаю, почему этот метод вообще тут, он ведь никак не работает с окружением
SimpleCloudLayout. По идее просто этот код надо перенести в место вызова этого метода и удалить его отсюда и из интерфейса
| public IPointGenerator CreatePointGenerator(LogicSettings logicSettings) => | ||
| logicSettings.PointGeneratorType switch | ||
| { | ||
| PointGeneratorType.Astroid => new AstroidPointGenerator(logicSettings), | ||
| PointGeneratorType.Spiral => new SpiralPointGenerator(logicSettings), | ||
| _ => throw new ArgumentOutOfRangeException(nameof(logicSettings.PointGeneratorType), | ||
| logicSettings.PointGeneratorType, | ||
| "No such pointGenerator type.") | ||
| }; |
There was a problem hiding this comment.
При появлении нового IPointGenerator придется добавлять его в фабрику, не очень будет удобно это делать, и вроде бы можно этого избежать. Давай в месте вызова этой фабрики будет получать массив IPointGenerator из контейнера и выбирать нужный по переданному типу (скорее всего придется добавить в интерфейс ещё один метод, который бы помогал определять, поддерживает ли генератор тот или иной переданный тип генерации из опций, фабрика тогда вообще перестанет быть нужна
| { | ||
| var result = new List<IWordTag>(); | ||
| const int rectangleOutline = 1; | ||
| var cloudLayout = new SimpleCloudLayout(logicSettings, new SimplePointGeneratorFactory()); |
There was a problem hiding this comment.
А почему лейаут создаем руками, а не через конструктор? Это ведь тоже зависимость, которую можем зарегать
| var result = new List<IWordTag>(); | ||
| const int rectangleOutline = 1; | ||
| var cloudLayout = new SimpleCloudLayout(logicSettings, new SimplePointGeneratorFactory()); | ||
| var bitmap = new Bitmap( |
|
|
||
| namespace TagCloudTests.SizeGenerator; | ||
|
|
||
| public class RandomSizesGenerator : ISizesGenerator |
There was a problem hiding this comment.
Используется в рамках одного тест-класса, можно просто добавить приватный метод, который будет делать то же самое и тем самым избавиться от лишнего интерфейса и класса
| private static readonly IFileReader Reader = new SingleWordInRowFileReader(); | ||
|
|
||
| [Test] | ||
| public void Handle_ShouldReturnWordsInLowerCase_AfterExecutionWithUpperCaseWords() |
There was a problem hiding this comment.
Тут вроде подмешал альтернативный нейминг - класс называется с Should, но в то же время Should есть и в тест-методе
| var oneWordCollection = new List<string> | ||
| { | ||
| "ясно", "ясно", "ясно", "ясно", "ясно", "ясно" | ||
| }; | ||
| var expectedNumberOfWords = oneWordCollection.GroupBy(x => x).Count(); |
There was a problem hiding this comment.
Это немного лишняя нагрузка, можешь сразу создать словарь и в нем написать ["ясно"] = 6
| imageSettingsProvider.SetWidth(updatedSettings?.Width ?? 1000); | ||
| imageSettingsProvider.SetHeight(updatedSettings?.Height ?? 1000); | ||
| imageSettingsProvider.SetFontFamily(updatedSettings?.FontFamily ?? new FontFamily("Arial")); | ||
| imageSettingsProvider.SetMaxFontSize(updatedSettings?.MaxFontSize ?? 24); | ||
| imageSettingsProvider.SetMinFontSize(updatedSettings?.MinFontSize ?? 8); |
| logicSettingsProvider.SetAngleStep(updatedSettings?.AngleStep ?? 0.01); | ||
| logicSettingsProvider.SetRadiusStep(updatedSettings?.RadiusStep ?? 0.01); | ||
| logicSettingsProvider.SetPointGenerator(updatedSettings?.PointGeneratorType ?? PointGeneratorType.Spiral); |
|
|
||
| public class SimpleWordHandler(IFileReader reader) : IWordHandler | ||
| { | ||
| private const string BoringWordsFilePath = "BoringWordsDictionary.txt"; |
There was a problem hiding this comment.
А как пользователь может влиять на список этих слов, если не захочет, например, выводить слово "Контур"?
@desolver