-
Notifications
You must be signed in to change notification settings - Fork 306
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Черных Илья #243
base: master
Are you sure you want to change the base?
Черных Илья #243
Changes from 16 commits
9590cf2
ef5b9d0
c3361d0
9e42a71
bd1809c
0747ad9
f792052
c17e0f9
09efbed
2b2988d
5e32c12
7aa664f
42db42a
07fd1a3
0bd2619
2e4a6c9
f92ef27
5c9d9b7
353a7f5
21dfe52
66bfc9e
492bdc1
19ccfbf
ad8646b
e736a26
66c278f
61330d3
c35b934
d9abf43
3274439
d3f04e6
44309c3
938e8d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,48 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public class CircularCloudLayouter : ICircularCloudLayouter | ||
{ | ||
private readonly Point center; | ||
private readonly List<Rectangle> addedRectangles; | ||
private double currentAngleOfSpiral; | ||
private double currentRadiusOfSpiral; | ||
|
||
public CircularCloudLayouter(Point center) | ||
{ | ||
this.center = center; | ||
addedRectangles = new List<Rectangle>(); | ||
} | ||
|
||
public Rectangle PutNextRectangle(Size rectangleSize) | ||
{ | ||
Rectangle rectangle; | ||
|
||
do | ||
{ | ||
var x = center.X + (int)(currentRadiusOfSpiral * Math.Cos(currentAngleOfSpiral)) - rectangleSize.Width / 2; | ||
var y = center.Y + (int)(currentRadiusOfSpiral * Math.Sin(currentAngleOfSpiral)) - rectangleSize.Height / 2; | ||
rectangle = new Rectangle(new Point(x, y), rectangleSize); | ||
|
||
// увеличиваем угол на 1 градус | ||
currentAngleOfSpiral += Math.PI / 180; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Тут и в ифке ниже явно прослеживается какая-то константа, которую можно было бы вынести отдельно, чтобы не высчитывать каждый раз |
||
|
||
// проверяем не прошли ли целый круг | ||
if (currentAngleOfSpiral > 2 * Math.PI) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. У тебя тут получилась не совсем спираль, т.к. ты сначала пытаешься расположить прямоугольники по периметру круга, и только если по периметру укладывать больше нельзя увеличиваешь радиус, тем самым создавая новый круг. Я не требую что-то менять в логике, но нейминг точно стоит поправить, потому что он не соответствует алгоритму |
||
{ | ||
currentAngleOfSpiral = 0; | ||
currentRadiusOfSpiral++; | ||
} | ||
} while (IntersectWithAddedRectangles(rectangle)); | ||
|
||
addedRectangles.Add(rectangle); | ||
|
||
return rectangle; | ||
} | ||
|
||
private bool IntersectWithAddedRectangles(Rectangle rectangle) | ||
{ | ||
return addedRectangles.Any(addedRectangle => addedRectangle.IntersectsWith(rectangle)); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public interface ICircularCloudLayouter | ||
{ | ||
Rectangle PutNextRectangle(Size rectangleSize); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
![Визуализация десяти прямоугольников](images/visualization10Rectangles.png) | ||
![Визуализация пятидесяти прямоугольников](images/visualization50Rectangles.png) | ||
![Визуализация ста прямоугольников](images/visualization100Rectangles.png) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="System.Drawing.Common" Version="6.0.0" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<Folder Include="images\" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,39 @@ | ||
using System.Drawing; | ||
using System.Drawing.Imaging; | ||
|
||
namespace TagsCloudVisualization; | ||
#pragma warning disable CA1416 | ||
|
||
public class VisualizationCircularCloudLayout | ||
{ | ||
public static void DrawAndSaveLayout(List<Rectangle> rectangles, string fileName, string filePath) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. По названию можно заметить, что метод выполняет сразу 2 функции: генерирует графическое представление нашего облака и сохраняет его в какой-то файл по переданному пути и имени. Такого в идеале быть не должно, у одного метода должно быть одно назначение. Так что предлагаю разбить на две части: одна вернет графическое представление, другая - сохранит его в файл |
||
{ | ||
const int padding = 10; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Эту константу можно передавать параметром или в метод или в конструктор, чтобы можно было отступ кастомизировать |
||
var minX = rectangles.Min(rectangle => rectangle.Left); | ||
var minY = rectangles.Min(rectangle => rectangle.Top); | ||
var maxX = rectangles.Max(rectangle => rectangle.Right); | ||
var maxY = rectangles.Max(rectangle => rectangle.Bottom); | ||
var width = maxX - minX + padding * 2; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Мелочь, но если отступ мы ужа задали, то довольно странно его потом умножать на 2, не лучше ли сразу указать нужный размер? |
||
var height = maxY - minY + padding * 2; | ||
|
||
var random = new Random(); | ||
|
||
var bitmap = new Bitmap(width, height); | ||
using var graphics = Graphics.FromImage(bitmap); | ||
|
||
graphics.Clear(Color.White); | ||
|
||
foreach (var rectangle in rectangles) | ||
{ | ||
var shiftedRectangle = rectangle with { X = rectangle.X - minX + padding, Y = rectangle.Y - minY + padding }; | ||
|
||
var randomColor = Color.FromArgb(random.Next(256), random.Next(256), random.Next(256)); | ||
|
||
var brush = new SolidBrush(randomColor); | ||
graphics.FillRectangle(brush, shiftedRectangle); | ||
graphics.DrawRectangle(Pens.Black, shiftedRectangle); | ||
} | ||
|
||
bitmap.Save(Path.Combine(filePath, fileName), ImageFormat.Png); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Формат изображения тоже было бы хорошо передавать параметром, чтобы пользователь мог сам выбирать, в каком формате его сохранять |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
using System.Drawing; | ||
using FluentAssertions; | ||
using NUnit.Framework; | ||
using NUnit.Framework.Interfaces; | ||
using TagsCloudVisualization; | ||
|
||
namespace Tests; | ||
|
||
[TestFixture] | ||
public class CircularCloudLayouterTests | ||
{ | ||
private ICircularCloudLayouter cloudLayouter; | ||
private List<Rectangle> addedRectangles; | ||
|
||
[SetUp] | ||
public void Setup() | ||
{ | ||
cloudLayouter = new CircularCloudLayouter(new Point(0, 0)); | ||
addedRectangles = []; | ||
} | ||
|
||
[TearDown] | ||
public void TearDown() | ||
{ | ||
if (TestContext.CurrentContext.Result.Outcome.Status != TestStatus.Failed) | ||
return; | ||
|
||
var pathImageStored = TestContext.CurrentContext.TestDirectory + @"\imageFailedTests"; | ||
|
||
if (!Directory.Exists(pathImageStored)) | ||
{ | ||
Directory.CreateDirectory(pathImageStored); | ||
} | ||
|
||
var testName = TestContext.CurrentContext.Test.Name; | ||
|
||
VisualizationCircularCloudLayout.DrawAndSaveLayout(addedRectangles, $"{testName}.png", | ||
pathImageStored); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Если переносишь один параметр в методе, то лучше переносить сразу все |
||
|
||
Console.WriteLine($@"Tag cloud visualization saved to file {pathImageStored}\{testName}.png"); | ||
} | ||
|
||
[Test] | ||
public void PutNextRectangle_ShouldRectangleInCenter_WhenAddFirstRectangle() | ||
{ | ||
var rectangleSize = new Size(10, 15); | ||
|
||
var addedRectangle = cloudLayouter.PutNextRectangle(rectangleSize); | ||
var expectedRectangleStartPoint = new Point(-addedRectangle.Width / 2, - addedRectangle.Height / 2); | ||
|
||
addedRectangle.Location.Should().BeEquivalentTo(expectedRectangleStartPoint); | ||
} | ||
|
||
[TestCase(10, 5, 15)] | ||
[TestCase(50, 30, 100)] | ||
[TestCase(100, 5, 50)] | ||
public void PutNextRectangle_ShouldAddedRectanglesDoNotIntersect(int countRectangles, int minSideLength, | ||
int maxSideLength) | ||
{ | ||
var rectangleSizes = GetGeneratedRectangleSizes(countRectangles, minSideLength, maxSideLength); | ||
|
||
addedRectangles.AddRange(rectangleSizes.Select(t => cloudLayouter.PutNextRectangle(t))); | ||
|
||
for (var i = 0; i < addedRectangles.Count-1; i++) | ||
{ | ||
addedRectangles | ||
.Skip(i + 1) | ||
.Any(addedRectangle => addedRectangle.IntersectsWith(addedRectangles[i])) | ||
.Should() | ||
.BeFalse(); | ||
} | ||
} | ||
|
||
[TestCase(10, 5, 15)] | ||
[TestCase(50, 30, 100)] | ||
[TestCase(100, 5, 50)] | ||
public void CircleShape_ShouldBeCloseToCircle_WhenAddMultipleRectangles(int countRectangles, int minSideLength, | ||
int maxSideLength) | ||
{ | ||
var rectangleSizes = GetGeneratedRectangleSizes(countRectangles, minSideLength, maxSideLength); | ||
|
||
addedRectangles.AddRange(rectangleSizes.Select(t => cloudLayouter.PutNextRectangle(t))); | ||
|
||
var distances = addedRectangles | ||
.Select(rectangle => GetDistanceBetweenCenterRectangleAndCenterCloud(rectangle, new Point(0, 0))) | ||
.ToArray(); | ||
|
||
for (var i = 1; i < distances.Length; i++) | ||
{ | ||
var distanceBetweenRectangles = GetDistanceBetweenRectangles(addedRectangles[i], addedRectangles[i - 1]); | ||
distances[i].Should().BeApproximately(distances[i - 1], distanceBetweenRectangles); | ||
} | ||
} | ||
|
||
|
||
private static List<Size> GetGeneratedRectangleSizes(int countRectangles, int minSideLength, int maxSideLength) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Get тут не очень хорошо подходит, т.к. он подразумевает, что мы откуда-то что-то берем, когда на самом деле мы вычисляем (других методов тоже касается) |
||
{ | ||
var generatedSizes = new List<Size>(); | ||
var random = new Random(); | ||
|
||
for (var i = 0; i < countRectangles; i++) | ||
{ | ||
var rectangleSize = new Size(random.Next(minSideLength, maxSideLength), | ||
random.Next(minSideLength, maxSideLength)); | ||
|
||
generatedSizes.Add(rectangleSize); | ||
} | ||
|
||
return generatedSizes; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Как будто весь метод можно свернуть в компактную Linq-цепочку |
||
|
||
private static double GetDistanceBetweenCenterRectangleAndCenterCloud(Rectangle rectangle, Point center) | ||
{ | ||
var centerRectangle = new Point(rectangle.X + rectangle.Width / 2, rectangle.Y + rectangle.Height / 2); | ||
|
||
return GetDistanceBetweenPoints(centerRectangle, center); | ||
} | ||
|
||
private static double GetDistanceBetweenRectangles(Rectangle rectangle1, Rectangle rectangle2) | ||
{ | ||
var centerRectangle1 = new Point(rectangle1.X + rectangle1.Width / 2, rectangle1.Y + rectangle1.Height / 2); | ||
var centerRectangle2 = new Point(rectangle2.X + rectangle2.Width / 2, rectangle2.Y + rectangle2.Height / 2); | ||
|
||
return GetDistanceBetweenPoints(centerRectangle1, centerRectangle2); | ||
} | ||
|
||
private static double GetDistanceBetweenPoints(Point point1, Point point2) | ||
{ | ||
return Math.Sqrt(Math.Pow(point1.X - point2.X, 2) + Math.Pow(point1.Y - point2.Y, 2)); | ||
} | ||
} |
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Мелочь, но названия проектов лучше всегда делать понятными, даже для тестовых или учебных заданий, это развивает привычку использовать хорошие называния |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="FluentAssertions" Version="7.0.0-alpha.5" /> | ||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" /> | ||
<PackageReference Include="NUnit" Version="4.2.2" /> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\TagsCloudVisualization\TagsCloudVisualization.csproj" /> | ||
</ItemGroup> | ||
|
||
</Project> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сейчас твой метод кладет прямоугольники всегда одним и тем же способом, но в идеале было бы хорошо алгоритм раскладки вынести куда-то в сторону и передавать его в конструктор, чтобы разные экземпляры CircularCloudLayouter могли раскладывать разными способами. Конкретно в этой задаче способ нужен только один, но на будущее лучше сделать класс гибче, и чтобы он почти ничего не знал о том, по какому алгоритму выбираются точки для прямоугольников