Копытов Михаил#266
Conversation
|
|
||
| public class CircularCloudLayouter | ||
| { | ||
| private List<Rectangle> Rects { get; } |
| public IEnumerable<Rectangle> GetCircularCloudRectangles(int count, Size rectangleSize, double coefficient = 0) | ||
| { | ||
| if (count <= 0) | ||
| throw new ArgumentException("Count must be positive"); | ||
| if (coefficient == 0) | ||
| { | ||
| for (var i = 0; i < count; i++) | ||
| yield return PutNextRectangle(rectangleSize); | ||
| yield break; | ||
| } | ||
|
|
||
| if (coefficient is < 0 or > 1) | ||
| throw new ArgumentException("Coefficient must be between 0 and 1"); | ||
| for (var i = 0; i < count; i++) | ||
| { | ||
| var rnd = random.NextDouble() + coefficient; | ||
| var rndWidth = (int)Math.Round(rectangleSize.Width * rnd); | ||
| var rndHeight = (int)Math.Round(rectangleSize.Height * rnd); | ||
| yield return PutNextRectangle(new Size(rndWidth, rndHeight)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Лучше вынести из лейаутера во внешний провайдер прямоугольников (или позиций прямоугольников)
|
|
||
| public static class ImageSaver | ||
| { | ||
| public static void ImageSave(Bitmap bitmap, string fileName) |
|
|
||
| namespace TagsCloudVisualization; | ||
|
|
||
| public class PointGenerator |
| if (center.X < 0 || center.Y < 0) | ||
| throw new ArgumentException("Center coordinates must be non-negative"); | ||
| if (imageSize.Width <= 0 || imageSize.Height <= 0) | ||
| throw new ArgumentException("Image size must be positive"); |
There was a problem hiding this comment.
Эти проверки повторяются в двух местах, может их лучше оставить только там, где эти параметры используются?
| yield break; | ||
| } | ||
|
|
||
| if (coefficient is < 0 or > 1) |
There was a problem hiding this comment.
Что за коэффициент? Из названия не понятно?
|
|
||
| namespace TagsCloudVisualization; | ||
|
|
||
| public class Render(Size imageSize) |
There was a problem hiding this comment.
Render -> Renderer или TagCloudRenderer. Необходимо вынести интерфейс ICloudRenderer
|
|
||
| act.Should().Throw<ArgumentException>() | ||
| .WithMessage("Center should be greater than 0"); | ||
| ; |
|
|
||
| act.Should().Throw<ArgumentException>() | ||
| .WithMessage("Center must be inside image bounds"); | ||
| ; |
There was a problem hiding this comment.
Здесь стоит добавить тест на то что прямоугольники распологаются по форме круга, а также на их плотность расположения
| if (center.X <= 0 || center.Y <= 0) | ||
| throw new ArgumentException("Center should be greater than 0"); | ||
| Rectangles = new List<Rectangle>(); | ||
| spiralPointGenerator = new SpiralPointGenerator(center); |
There was a problem hiding this comment.
Зависимости должны поставляться через конструктор. Работа должна проводиться через интерфейс, а не напрямую с классом
There was a problem hiding this comment.
Просто не хотелось отходить от первоначальной постановки задачи, где требуется, чтобы конструктор принимал только значение центра
|
|
||
| private int maxPointCount = 10000; | ||
|
|
||
| public void SetMaxPointCount(int maxPointCount) |
There was a problem hiding this comment.
Зачем тут торчит отдельный метод? Почему это значение не устанавливать в конструкторе?
|
|
||
| var points = spiralPointGenerator | ||
| .GetPoints() | ||
| .Take(maxPointCount); |
There was a problem hiding this comment.
Кажется для maxPointCount стоит уточнить нейминг. По факту это точки, которые возьмём на один прямоугольник.
| foreach (var point in points) | ||
| { | ||
| var rectangle = new Rectangle(point, rectangleSize); | ||
| if (Rectangles.Any(rectangle.IntersectsWith)) |
There was a problem hiding this comment.
На самом деле тут имеет смысл смотреть пересечения от последних раставленных прямоугольников, т.к. они ближе всего будут к потенциальному расположению прямоугольника. Так более часто будем проверять меньшее количество прямоугольников
|
|
||
| public class CircularCloudLayouter | ||
| { | ||
| private List<Rectangle> Rectangles { get; } |
There was a problem hiding this comment.
Почему через геттер получаем, а не сделать просто readonly поле?
|
|
||
| public interface ICloudRenderer | ||
| { | ||
| public Bitmap CreateRectangleCloud(IEnumerable<Rectangle> rectangles); |
There was a problem hiding this comment.
Не совсем понял, что требуется
| using System.Drawing; | ||
| using System.Drawing.Imaging; | ||
|
|
||
| namespace TagsCloudVisualization.Render; |
|
|
||
| namespace TagsCloudVisualization.Render; | ||
|
|
||
| public static class ImageSaver |
| { | ||
| public static void Save(Bitmap bitmap, string fileName) | ||
| { | ||
| var projectDir = Directory.GetParent(Environment.CurrentDirectory)!.Parent!.Parent!.FullName; |
There was a problem hiding this comment.
А если нет такой вложенности? Если я запущу это из коренной папки что будет?
|
|
||
| public interface IPointGenerator | ||
| { | ||
| public IEnumerable<Point> GetPoints(); |
|
|
||
| namespace TagsCloudVisualizationTest; | ||
|
|
||
| public class SpiralPointProviderTest |
There was a problem hiding this comment.
Такие тесты должны быть на CircularCloudLayouter, а не на класс провайдера.
| var circleRadius = Geometry.FindMinRadius(rectangles, validCenter); | ||
| var packingDensity = Geometry.GetRectanglesPackingDensity(rectangles, circleRadius); | ||
|
|
||
| packingDensity.Should().BeGreaterThan(0.4); |
There was a problem hiding this comment.
Не маловато ли значение? По-хорошему должно быть больше 0.6-0.7. Попробуй тут большее количество прямоугольников расположить
There was a problem hiding this comment.
Если сделать 500 прямоугольников, то 0.6 достигает, но работает медленно. Для 150 достигает 0.5. Но вроде визуально прямоугольники расположены достаточно плотно.
2 participants
@tripples25