Ситдиков Сергей#282
Conversation
| var newLocation = new Point(current.X + stepX, current.Y + stepY); | ||
| var newCandidate = new Rectangle(newLocation, current.Size); | ||
|
|
||
| if (!IntersectsWithAny(newCandidate) && !IsContainedInAny(newCandidate)) |
There was a problem hiding this comment.
Точно нужна проверка на пересечение здесь? В PutNextRectangle и так ведь она есть
|
|
||
| private bool IntersectsWithAny(Rectangle rectangle) => _rectangles.Any(r => r.IntersectsWith(rectangle)); | ||
|
|
||
| private bool IsContainedInAny(Rectangle candidate) |
There was a problem hiding this comment.
Не кажется что этот метод и IntersectsWithAny это одно и то же?
|
|
||
| public CircularCloudLayouter(Point center) | ||
| { | ||
| _center = center; |
There was a problem hiding this comment.
Давай нейминг поправим. Пусть будет center, и так для других полей. Обычно принято просто с маленькой буквы приватные называть
| private readonly List<Rectangle> _rectangles; | ||
| private readonly Spiral _spiral; | ||
|
|
||
| public CircularCloudLayouter(Point center) |
There was a problem hiding this comment.
Было бы удобно в этот класс настройки генерации передавать. Тот же шаг например
| if (rectangleSize.Width <= 0 || rectangleSize.Height <= 0) | ||
| throw new ArgumentException("The size of the rectangle must be a positive number."); | ||
|
|
||
| if (_rectangles.Count == 0) |
There was a problem hiding this comment.
Можно использовать Any или if (_rectangles is [])
| } | ||
| } | ||
|
|
||
| throw new InvalidOperationException("Can't place a rectangle."); |
There was a problem hiding this comment.
Этот код вообще будет достигнут когда-то?
| return newDistance < currentDistance; | ||
| } | ||
|
|
||
| private double DistanceToCenter(Rectangle rect) |
There was a problem hiding this comment.
Этот метод у тебя только для сравнения используется, реальное расстояние не нужно. Поэтому можно например корень не считать. А еще умножение быстрее Math.Pow будет работать
| else | ||
| break; | ||
|
|
||
| if (Math.Abs(current.X - candidate.X) > 100 || Math.Abs(current.Y - candidate.Y) > 100) |
There was a problem hiding this comment.
100 вынести в константу хотя бы? Или в какой-то общий класс с настройками генератора облака
| public Spiral(Point center, double step) | ||
| { | ||
| this.center = center; | ||
| this.step = step; |
There was a problem hiding this comment.
Может ли сюда попасть step <= 0 ?
| while (true) | ||
| { | ||
| var radius = step * angle; | ||
| var x = center.X + (int)(radius * Math.Cos(angle)); |
There was a problem hiding this comment.
Ожидаемое ли это поведение что тут каждый раз точность теряется при приведении к int?
| var maxHeight = Math.Max(maxTop, maxBottom) * 2 + 2 * padding; | ||
|
|
||
| var maxSide = Math.Max(maxWidth, maxHeight); | ||
| maxSide = Math.Max(maxSide, 400); |
There was a problem hiding this comment.
400 тоже какое-то магическое число, надо его куда-то вынести
|
|
||
| private void DrawRectangles(Graphics graphics, List<Rectangle> rectangles, Point offset) | ||
| { | ||
| using var pen = new Pen(rectangleColor, 1.5f); |
| var rectangle = layouter.PutNextRectangle(rectangleSize); | ||
| placedRectangles.Add(rectangle); | ||
|
|
||
| var rectanglePoint = new Point(center.X - rectangleSize.Width / 2, center.Y - rectangleSize.Height / 2); |
There was a problem hiding this comment.
Тут лишний пробел в строке. Отформатируй файлики плиз
| placedRectangles.Add(secondRectangle); | ||
|
|
||
| rectangle.Should().NotBeNull(); | ||
| secondRectangle.Should().NotBeNull(); |
There was a problem hiding this comment.
Разве Rectangle может быть null?
|
|
||
| for (int i = 1; i <= 10; i++) | ||
| { | ||
| var rect = layouter.PutNextRectangle(new Size(10 * i - i, 10 * i - i * 2)); |
There was a problem hiding this comment.
Не понятно откуда такая сложная формула взялась, и может ее упростить как-то?
| } | ||
|
|
||
| foreach (var r1 in rectangles) | ||
| foreach (var r2 in rectangles.Where(r2 => r2 != r1)) |
There was a problem hiding this comment.
В общем-то пофиг, но ты каждую пару дважды проверяешь. Сначала r1 с r2, потом наоборот
| var height = bounds.Height; | ||
|
|
||
| var aspectRatio = (double)Math.Max(width, height) / Math.Min(width, height); | ||
| aspectRatio.Should().BeLessThan(1.5); |
2 participants
@PeterMotorniy