Заколюкин Степан #255
Заколюкин Степан #255
Conversation
…конструктор класса
…иков, создание отчетов об ошибкаx
cs/TagsCloudVisualization/Program.cs
Outdated
| .Save("../../../../TagsCloudVisualization/Task 2/CentralСloud.png"); | ||
|
|
||
| visual.CreateAnImage(new CircularCloudLayouter(new Point(250, 150)), 50, 30, 80, 5, 25) | ||
| .Save("../../../../TagsCloudVisualization/Task 2/SmalСloud.png"); |
|
|
||
| <ItemGroup> | ||
| <Folder Include="TestErrorReports\" /> | ||
| </ItemGroup> |
| public int DistanceBetweenTurns { get; private set; } | ||
| public int InitialRadiusOfTheSpiral { get; private set; } | ||
| public double AngleOfRotationInRadians { get; private set; } | ||
| public double AngleChangeStep { get; private set; } |
There was a problem hiding this comment.
Не увидел чтобы свойство менялось, можно убрать сеттер
There was a problem hiding this comment.
Да и вообще лучше в константу вынести и убрать публик, тк это клиенту не надо, а ещё DistanceBetweenTurns, InitialRadius, AngleOfRotation. Это всё - реализация, клиенту не упало
Center в теории может пригодиться, наверное, можно оставить публичным
| { | ||
| public static IEnumerable<Rectangle> GenerateCloudLayout(int amountRectangles, | ||
| int startWidthRectangles, int endWidthRectangles, | ||
| int startHeightRectangles, int endHeightRectangles, |
There was a problem hiding this comment.
докопаюсь до нейминга. Лучше писать startRectangleWidth
А ещё есть Size, который хранит width и height, выйдет Size minSize, Size maxSize
There was a problem hiding this comment.
Ещё сюда бы прикрутить валидацию, что min < max
| return rectangle; | ||
| } | ||
|
|
||
| private Rectangle GetANewRectangle(Point centerPoint, Size rectangleSize) => |
| InitialRadiusOfTheSpiral = Math.Min(rectangleSize.Width, rectangleSize.Height) / 2; | ||
|
|
||
| DistanceBetweenTurns = Math.Min(DistanceBetweenTurns, | ||
| Math.Min(rectangleSize.Width, rectangleSize.Height) / 2); |
There was a problem hiding this comment.
ты 2 раза высчитываешь это, стоит вынести в переменную
| Center = center; | ||
| AngleChangeStep = 0.017; | ||
| cloudOfRectangles = []; | ||
| DistanceBetweenTurns = 30; |
There was a problem hiding this comment.
Я не знаю почему это 30, наверное, стоит это вынести в конструктор чтобы абстрактный клиент мог это настроить
There was a problem hiding this comment.
Вынести то можно, просто это значение потом все равно изменяется в методе PutNextRectangle в зависимости от самого маленького размера, который когда либо поступал в метод, это сделано для того чтобы увеличить кучность облака. Исходя из этого, я подумал, что наверное не будет смысла в возможности пользователя указать какое-то значение в конструкторе, т.к класс будет впоследствии его менять по определенному правилу
There was a problem hiding this comment.
ну я так понял оно будет меняться только в меньшую сторону, и, как ты сказал, оно влияет на кучность. Может, пользователь захочет облако побольше и чтобы между тегами было расстояние
There was a problem hiding this comment.
как бы в целом, для решения задачи создания облака слов это справедливо, но если говорить конкретно об задаче из домашки, то там был такой критерий: "Облако должно быть плотным, чем плотнее, тем лучше."
| @@ -17,16 +17,17 @@ public VisualizationCloudLayout(int width, int height, Color backgroundColor, IE | |||
| RectanglePalette = rectanglePalette.ToArray(); | |||
| } | |||
|
|
|||
| public Bitmap CreateAnImage(Point center, int amountRectangles, | |||
| public Bitmap CreateAnImage(CircularCloudLayouter cloudLayouter, int amountRectangles, | |||
| public class CircularCloudLayouterTests | ||
| { | ||
| private readonly static List<Rectangle> listRectangles = []; | ||
| private readonly static CircularCloudLayouter cloudLayouter; |
There was a problem hiding this comment.
и статик не нужен
И я не понял как это работает: readonly поле не задаётся, но в конце TearDown его пробрасывает и вызывает
There was a problem hiding this comment.
Это поле тут оказалась лишним, оно не использовалось для создания визуализации, я добавил перегрузку метода и убрал его
| public readonly int Width; | ||
| public readonly int Height; | ||
| public readonly Color BackgroundColor; | ||
| public readonly Color[] RectanglePalette; |
There was a problem hiding this comment.
Тут тоже публичность надо убрать, клиенту это не интересно
| var path = "../../../../TagsCloudVisualization/TestErrorReports/сloud.png"; | ||
| var visual = new VisualizationCloudLayout(1920, 1080, Color.White, colors); | ||
|
|
||
| visual.CreateAnImage(cloudLayouter, 175, 30, 100, 5, 25, listRectangles) |
There was a problem hiding this comment.
Вообще по хорошему следует передавать не layouter, а результат его работы, тк TearDown это уже доп операции с отчетом о тестах, отрубом баз данных, если были использованы, итд.
И в принципе передавать в аргументы объект, который совершает действие, это не хорошо, такого следует избегать. Делегаты - "вы не понимаете, это другое", тк там чаще всего передают функцию, совершающую мелкие и сходу понятные действия типа фильтрации или аггрегирования
@Inree