Овечкин Илья #220
Овечкин Илья #220
Conversation
magnat0
changed the title
| { | ||
| public class CircularCloudLayouter | ||
| { | ||
| private List<Rectangle> rectangles; |
There was a problem hiding this comment.
Лучше ICollection, если возможно. Это позволит при необходимости поменять реализацию не опасаясь потери обратной совместимости
| public class CircularCloudLayouter | ||
| { | ||
| private List<Rectangle> rectangles; | ||
| private Point center; |
There was a problem hiding this comment.
Никогда не присваивается, мб и не нужно
| private List<Rectangle> rectangles; | ||
| private Point center; | ||
| private Spiral spiral; |
| return nextRengtanle; | ||
| } | ||
|
|
||
| public List<Rectangle> Rectangles => rectangles; |
There was a problem hiding this comment.
Лучше IReadonlyCollection. Это защитить от опасности редактирования листа кем-нибудь снаружи
cs/TagsCloudVisualization/Spiral.cs
Outdated
|
|
||
| namespace TagsCloudVisualization | ||
| { | ||
| public class Spiral |
There was a problem hiding this comment.
Мне нравится идея выделения логики генерации прямоугольников от места их хранения, но немного смущает название класса. Хорошо бы как-то более точно отобразить его роль в названии
(тут же маленькая опциональная рекомендация - делать все свои классы sealed, если не требуется обратное - такой код работает чуточку быстрее + дважды заставит задуматься, а стоит ли наследоваться от данного класса)
cs/TagsCloudVisualization/Spiral.cs
Outdated
|
|
||
| while (true) | ||
| { | ||
| Point point = new Point( |
| graphics = Graphics.FromImage(image); | ||
| } | ||
|
|
||
| public void drawCloud() |
There was a problem hiding this comment.
Тут конвенция именования пошла куда-то не туда. Должен быть PascalCase
| public void drawCloud() | ||
| { | ||
| graphics.Clear(Color.White); | ||
| foreach (var rect in layouter.Rectangles) | ||
| { | ||
| graphics.FillRectangle(Brushes.Red, rect); | ||
| graphics.DrawRectangle(Pens.Black, rect); | ||
| } | ||
| graphics.Save(); | ||
|
|
||
| } | ||
|
|
||
| public void saveImage(string fileName) | ||
| { | ||
| image.Save(fileName); | ||
| } |
There was a problem hiding this comment.
Есть ли необходимость разделять эти две функции, если они всегда вызываются в паре
| private Bitmap image; | ||
| private Graphics graphics; |
There was a problem hiding this comment.
Кажется, не должно быть в полях класса
| SaveIncorrectResultsToJpg(); | ||
| } | ||
|
|
||
| public static IEnumerable TestCasesForWrongCenterPoint |
There was a problem hiding this comment.
Нет необходимости превращать в перечисление. Можно сделать обычным массивом
@a_zhelonkin