Skip to content
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

Сахабутдинов Ильфир #240

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
150 changes: 150 additions & 0 deletions cs/TagsCloudVisualization/CircularCloudLayouter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,150 @@
using System.Drawing;

namespace TagsCloudVisualization;

public class CircularCloudLayouter : ICircularCloudLayouter

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ходят легенды, что если помечать классы sealed (особенно при большом количестве типов), можно получить неощутимые рост производительности. А если серьезно, то если класс не спроектирован под расширение, то лучше сразу запечатывать. Если понадобится, всегда можно будет открыть

{
private readonly IList<Rectangle> rectangles;
private readonly Point center;
private int layer;
private double angle;
private const double fullCircleTurn = Math.PI * 2;
private const int distanceLayersDifference = 1;
private const double betweenAngleDifference = Math.PI / 36; // 15°

public CircularCloudLayouter(Point center)
{
this.center = center;
rectangles = new List<Rectangle>();

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Возможно, это какая-то привычка, но всё это и ниже можно либо писать сразу напротив поля, либо и так значения по умолчанию

layer = 0;
angle = 0;
}

public Rectangle PutNextRectangle(Size rectangleSize)
{
if (rectangleSize.Width == 0 || rectangleSize.Height == 0)
{
throw new ArgumentException("Размер ширины м высоты должен быть больше 0.");
}

var rectangle = CreateNewRectangle(rectangleSize);
rectangle = RectangleCompressions(rectangle);
rectangles.Add(rectangle);
return rectangle;
}

public IReadOnlyCollection<Rectangle> GetRectangles()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Круто что используешь IReadonly*
nit: оборачивать в AsReadonly нет большой необходимости, достаточно того что произойдет сужение типа и уже только поэтому снаружи никто не сможет мутировать коллекцию. Конечно, технически все еще будет возможность привести тип обратно, но в таком случае это будет уже проблемой того самого разработчика. Ну и как следствие. можно перейти от функции к свойству

{
return rectangles.AsReadOnly();
}

private Rectangle CreateNewRectangle(Size rectangleSize)
{
var rectangleLocation = GetRectangleLocation(rectangleSize);
var rectangle = new Rectangle(rectangleLocation, rectangleSize);
while (CheckRectangleOverlaps(rectangle))
{
UpdateAngle();
rectangleLocation = GetRectangleLocation(rectangleSize);
rectangle = new Rectangle(rectangleLocation, rectangleSize);
}

return rectangle;
}

private Rectangle RectangleCompressions(Rectangle rectangle)
{
var compressionRectangle = rectangle;
compressionRectangle = Compression(compressionRectangle,
(moveRectangle) => moveRectangle.X > center.X,
(moveRectangle) => moveRectangle.X + rectangle.Width < center.X,
(moveRectangle, direction) => moveRectangle with {X = moveRectangle.X + direction});

compressionRectangle = Compression(compressionRectangle,
(moveRectangle) => moveRectangle.Y > center.Y,
(moveRectangle) => moveRectangle.Y + moveRectangle.Height < center.Y,
(moveRectangle, direction) => moveRectangle with {Y = moveRectangle.Y + direction});

return compressionRectangle;
}

private Rectangle Compression(Rectangle rectangle,
Func<Rectangle, bool> checkPositiveMove,
Func<Rectangle, bool> checkNegativeMove,
Func<Rectangle, int, Rectangle> doMove)
{
if (checkPositiveMove(rectangle) && checkNegativeMove(rectangle)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: если не ошибаюсь, можно свернуть в checkPositiveMove(rectangle) == checkNegativeMove(rectangle)

|| !checkPositiveMove(rectangle) && !checkNegativeMove(rectangle))
{
return rectangle;
}

var direction = checkPositiveMove(rectangle) ? -1 : 1;
var moveRectangle = rectangle;
while (true)
{
moveRectangle = doMove(moveRectangle, direction);
if (CheckRectangleOverlaps(moveRectangle))
{
moveRectangle = doMove(moveRectangle, -1 * direction);
break;
}

if ((direction == -1 && !checkPositiveMove(moveRectangle))
|| (direction == 1 && !checkNegativeMove(moveRectangle)))
{
break;
}
}

return moveRectangle;
}

private bool CheckRectangleOverlaps(Rectangle rectangle)
{
return rectangles.Any(r => r.IntersectsWith(rectangle));
}

private Point GetRectangleLocation(Size rectangleSize)
{
var shiftFromCenter = -1 * rectangleSize / 2;
if (!rectangles.Any())

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: холиварный момент, но часто рекомендую явно проверять размер, когда это возможно и не создает накладных расходов, иначе внутри будет создан лишний итератор, который попробует взять первый элемент. Последние версии dotnet'а научились это решать, но тем не менее

{
UpdateLayer();
return center + shiftFromCenter;
}

return CalculateLocationInSpiral() + shiftFromCenter;
}

private Point CalculateLocationInSpiral()
{
if (!rectangles.Any())
{
throw new ArgumentException(
"Должен быть хотя бы один прямоугольник для вычисления позиции для следующего вне центра спирали.");
}

var x = center.X + layer * distanceLayersDifference * Math.Cos(angle);
var y = center.Y + layer * distanceLayersDifference * Math.Sin(angle);
var wholeX = Convert.ToInt32(x);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: можно и просто кастонуть wholeX = (int) x. Внутри Convert происходит примерно то же самое

var wholeY = Convert.ToInt32(y);
var newLocation = new Point(wholeX, wholeY);
return newLocation;
}

private void UpdateAngle()
{
angle += betweenAngleDifference;
if (angle > fullCircleTurn)
{
UpdateLayer();
angle = 0;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Думаю, надежнее делать angle %= fullCircleTurn или хотя бы angle -= fullCircleTurn на случай, если в fullCircleTurn будет нецелое количество betweenAngleDifference

}
}

private void UpdateLayer()
{
layer++;
}
}
12 changes: 12 additions & 0 deletions cs/TagsCloudVisualization/Extensions/RectangleExtensions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
using System.Drawing;

namespace TagsCloudVisualization.Extensions;

public static class RectangleExtensions
{
public static Point Center(this Rectangle rectangle)
{
return new Point(rectangle.Left + rectangle.Width / 2,
rectangle.Top + rectangle.Height / 2);
}
}
9 changes: 9 additions & 0 deletions cs/TagsCloudVisualization/ICircularCloudLayouter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Drawing;

namespace TagsCloudVisualization;

public interface ICircularCloudLayouter
{
public Rectangle PutNextRectangle(Size rectangleSize);
public IReadOnlyCollection<Rectangle> GetRectangles();
}
10 changes: 10 additions & 0 deletions cs/TagsCloudVisualization/ITagsCloudVisualizer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
using System.Drawing;
using System.Drawing.Imaging;

namespace TagsCloudVisualization;

public interface ITagsCloudVisualizer : IDisposable
{
public void AddVisualizationRectangles(IEnumerable<Rectangle> rectangles);
public void Save(string outputFilePath, ImageFormat imageFormat);
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 5 additions & 0 deletions cs/TagsCloudVisualization/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
### Сгенерированные изображения с разными параметрами для примера
1) [100RectanglesFrom50To50Sizes.png](https://github.com/ILFirV-V/tdd/blob/master/cs/TagsCloudVisualization/Images/100RectanglesFrom50To50Sizes.png)
2) [100RectanglesFrom50To100Sizes.png](https://github.com/ILFirV-V/tdd/blob/master/cs/TagsCloudVisualization/Images/100RectanglesFrom50To100Sizes.png)
3) [100RectanglesFrom75To100Sizes.png](https://github.com/ILFirV-V/tdd/blob/master/cs/TagsCloudVisualization/Images/100RectanglesFrom75To100Sizes.png)
4) [100RectanglesWithWidthFrom50To75AndHeightFrom25To50Sizes.png](https://github.com/ILFirV-V/tdd/blob/master/cs/TagsCloudVisualization/Images/100RectanglesWithWidthFrom50To75AndHeightFrom25To50Sizes.png)
20 changes: 20 additions & 0 deletions cs/TagsCloudVisualization/TagsCloudVisualization.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<Project Sdk="Microsoft.NET.Sdk">

<PropertyGroup>
<TargetFramework>net8.0</TargetFramework>
<ImplicitUsings>enable</ImplicitUsings>
<Nullable>enable</Nullable>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="FluentAssertions" Version="6.12.2" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.11.1" />
<PackageReference Include="NUnit" Version="4.2.2" />
<PackageReference Include="System.Drawing.Common" Version="8.0.10" />
</ItemGroup>

<ItemGroup>
<Folder Include="Images\" />
</ItemGroup>

</Project>
86 changes: 86 additions & 0 deletions cs/TagsCloudVisualization/TagsCloudVisualizer.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
using System.Drawing;
using System.Drawing.Imaging;
using TagsCloudVisualization.Extensions;

namespace TagsCloudVisualization;

public class TagsCloudVisualizer : ITagsCloudVisualizer
{
private bool isDisposed;
private readonly Bitmap bitmap;

public TagsCloudVisualizer(Size imageSize)
{
bitmap = new Bitmap(imageSize.Width, imageSize.Height);
isDisposed = false;
}

public void AddVisualizationRectangles(IEnumerable<Rectangle> rectangles)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Выглядит норм, но кажется что перестарался с декомпозицией. Я бы сложил всё что касается graphics в одну функцию

{
using var graphics = Graphics.FromImage(bitmap);
VisualizeRectangles(graphics, rectangles);
VisualizeCenter(graphics, rectangles.First());
}

public void Save(string outputFilePath, ImageFormat imageFormat)
{
bitmap.Save(outputFilePath, imageFormat);
}

private void VisualizeRectangles(Graphics graphics, IEnumerable<Rectangle> rectangles)
{
foreach (var rectangle in rectangles)
{
VisualizeRectangle(graphics, rectangle);
}
}

private void VisualizeRectangle(Graphics graphics, Rectangle rectangle)
{
graphics.FillRectangle(Brushes.Blue, rectangle);
graphics.DrawRectangle(Pens.Black, rectangle);
}

private void VisualizeCenter(Graphics graphics, Rectangle firstRectangle)
{
var center = firstRectangle.Center();
var centerRectangle = CalculateCentralRectangle(center);
VisualizeCenterPoint(graphics, centerRectangle);
}

private Rectangle CalculateCentralRectangle(Point center)
{
var size = new Size(2, 2);
return new Rectangle(center.X - size.Width / 2, center.Y - size.Height / 2, size.Width, size.Height);
}

private void VisualizeCenterPoint(Graphics graphics, Rectangle centerRectangle)
{
graphics.FillEllipse(Brushes.Red, centerRectangle);
}

~TagsCloudVisualizer()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Зачем?

{
Dispose(false);
}

public void Dispose()
{
Dispose(true);
}

protected virtual void Dispose(bool fromDisposeMethod)
{
if (isDisposed)
{
return;
}

if (fromDisposeMethod)
{
bitmap.Dispose();
}

isDisposed = true;
}
}
38 changes: 38 additions & 0 deletions cs/TagsCloudVisualization/Tests/CircularCloudLayouterTestCases.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System.Drawing;
using NUnit.Framework;

namespace TagsCloudVisualization.Tests;

internal static class CircularCloudLayouterTestCases

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Есть вариант сделать этот класс и класс с тестами partial. Файл с кейсами назвать, например, CircularCloudLayouterTests.Cases.cs, чтобы было понятно, что 2 файла содержат общий тип. Тогда кейсы можно будет сделать приватными, а во втором классе можно будет избавиться от проксирования

{
internal static readonly IEnumerable<TestCaseData> GetRectanglesWithZeroSizesTestData =
[
new TestCaseData(new Point(0, 0), new Size(0, 0))
.SetName("AllZero"),
new TestCaseData(new Point(0, 0), new Size(1, 0))
.SetName("WidthZero"),
new TestCaseData(new Point(0, 0), new Size(0, 1))
.SetName("HeightZero"),
];

internal static readonly IEnumerable<TestCaseData> GetCorrectRectangleSizesWithEndsLocationTestData =
[
new TestCaseData(new Point(10, 10), new List<Size>() {new(10, 10)}, new Point(10, 10) - new Size(10, 10) / 2)
.SetName("FirstRectangleInCenter"),
];

internal static readonly IEnumerable<TestCaseData> GetCorrectUnusualRectanglesSizeTestData =
[
new TestCaseData(new Point(100, 100), Array.Empty<Size>())
.SetName("ArraySizeEmpty"),
];

internal static readonly IEnumerable<TestCaseData> GetCorrectRectangleSizesTestData =
[
new TestCaseData(new Point(100, 100), new List<Size>() {new(10, 10)})
.SetName("OneSize"),
new TestCaseData(new Point(100, 100), new List<Size>()
{new(10, 10), new(20, 20), new(15, 15), new(5, 7), new(3, 1), new(15, 35)})
.SetName("MoreSizes"),
];
}
Loading