-
Notifications
You must be signed in to change notification settings - Fork 307
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
Брозовский Максим #250
base: master
Are you sure you want to change the base?
Брозовский Максим #250
Changes from 18 commits
2038a71
f2196bf
f4ee9e4
58bc738
38fdceb
0f7a10d
8ff9eb8
0e12a6d
eb5be16
009dc35
3090cdb
6940721
04abe31
ba4e5f7
2a33e20
ff64502
9236bae
71ae050
f2bceec
e6c398f
c2cb66f
8a64693
5832415
f71a890
4f257ec
c74256f
5c25936
d7eda0b
6a2f944
1511372
87ebcc2
8a67c73
d18367e
0b1de9a
79f342d
687a820
901ffe5
b8ce60c
cdf1d62
dbd9352
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,47 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public class CircularCloudLayouter(Point center, IPointsGenerator pointsGenerator) : ICloudLayouter | ||
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. А зачем нам тут такой конструктор? Он, кажется, только усложняет код, т.к отличается от остальных. |
||
{ | ||
private readonly List<Rectangle> rectangles = new(); | ||
|
||
public Point Center => center; | ||
public IEnumerable<Rectangle> Rectangles => rectangles; | ||
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. А зачем нам это делать свойствами? Пусть полями внутри лежат. А то я могу взять и поменять координату, там же публичный сеттер у точки 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. Point - Struct, а значит это не проблема (он immutable), подробнее тут - https://stackoverflow.com/questions/3456554/points-properties-in-net |
||
|
||
public CircularCloudLayouter(Point center) : | ||
this(center, new SpiralPointsGenerator(center, 1d, 0.5d)) | ||
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. В константы |
||
{ | ||
|
||
} | ||
|
||
public CircularCloudLayouter(Point center, double radius, double angleOffset) : | ||
this(center, new SpiralPointsGenerator(center, radius, angleOffset)) | ||
{ | ||
|
||
} | ||
|
||
public Rectangle PutNextRectangle(Size rectangleSize) | ||
{ | ||
while (true) | ||
{ | ||
var rectanglePosition = pointsGenerator.GetNextPoint(); | ||
var rectangle = CreateRectangle(rectanglePosition, rectangleSize); | ||
|
||
if (rectangles.Any(rectangle.IntersectsWith)) continue; | ||
|
||
rectangles.Add(rectangle); | ||
|
||
return rectangle; | ||
} | ||
} | ||
|
||
public static Rectangle CreateRectangle(Point center, Size rectangleSize) => | ||
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. Я бы тут явно обозначил в названии, что мы не просто создаем прямоугольник, а создаем его с центром в точке center |
||
new( | ||
center.X - rectangleSize.Width / 2, | ||
center.Y - rectangleSize.Height / 2, | ||
rectangleSize.Width, | ||
rectangleSize.Height | ||
); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public interface ICloudLayouter | ||
{ | ||
public Rectangle PutNextRectangle(Size rectangleSize); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public interface IPointsGenerator | ||
{ | ||
public Point GetNextPoint(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
using System.Drawing; | ||
using SkiaSharp; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
class Program | ||
{ | ||
private const int ImageWidth = 2560; | ||
private const int ImageHeight = 1440; | ||
|
||
private const int NumberOfRectangles = 50; | ||
private const int MinRectangleSize = 10; | ||
private const int MaxRectangleSize = 50; | ||
|
||
private const string ImageDirectory = "../../../imgs"; | ||
public static void Main(string[] args) | ||
{ | ||
var center = new Point(ImageWidth / 2, ImageHeight / 2); | ||
var cloudLayouter = new CircularCloudLayouter(center); | ||
var randomizer = new Random(); | ||
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 rectangles = Enumerable | ||
.Range(0, NumberOfRectangles) | ||
.Select(_ => | ||
cloudLayouter.PutNextRectangle(randomizer.NextSize(MinRectangleSize, MaxRectangleSize))); | ||
|
||
var visualizer = new Visualizer(ImageWidth, ImageHeight); | ||
var bitmap = visualizer.VisualizeTagCloud(rectangles); | ||
|
||
var saver = new Saver(ImageDirectory); | ||
saver.SaveAsPng(bitmap, $"{NumberOfRectangles}_TagCloud.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,8 @@ | ||
### 50 прямоугольников | ||
<img src="imgs/50_TagCloud.png"> | ||
|
||
### 100 прямоугольников | ||
<img src="imgs/100_TagCloud.png"> | ||
|
||
### 1000 прямоугольников | ||
<img src="imgs/1000_TagCloud.png"> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public static class RandomExtension | ||
{ | ||
public static Size NextSize(this Random random, int minValue, int maxValue) | ||
{ | ||
if (minValue <= 0) | ||
throw new ArgumentOutOfRangeException(nameof(minValue), "minValue must be greater than 0"); | ||
if (maxValue < minValue) | ||
throw new ArgumentOutOfRangeException(nameof(maxValue), "maxValue must be greater than minValue"); | ||
|
||
return new Size(random.Next(minValue, maxValue), random.Next(minValue, maxValue)); | ||
} | ||
|
||
public static Point NextPoint(this Random random, int minValue, int maxValue) => | ||
new (random.Next(minValue, maxValue), random.Next(minValue, maxValue)); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
using SkiaSharp; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public class Saver(string imageDirectory) | ||
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. Saver чего? |
||
{ | ||
public void SaveAsPng(SKBitmap bitmap, string filename) | ||
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. А зачем нам на каждую директорию плодить объекты? Почему не можем сразу принять 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. Предлагаю посмотреть в сторону Path.GetDirectoryName(path), чтобы нам сразу явно передавали путь до файла, в т.ч относительный |
||
{ | ||
Directory.CreateDirectory(imageDirectory); | ||
using var file = File.OpenWrite(Path.Combine(imageDirectory, filename)); | ||
bitmap.Encode(SKEncodedImageFormat.Png, 80).SaveTo(file); | ||
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. Что такое 80? Как считается, откуда берется? |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public class SpiralPointsGenerator : IPointsGenerator | ||
{ | ||
private readonly double angleOffset; | ||
private readonly double radius; | ||
private readonly Point start; | ||
private double angle; | ||
|
||
public SpiralPointsGenerator(Point start, double radius, double angleOffset) | ||
{ | ||
if (radius <= 0) | ||
throw new ArgumentException("radius must be greater than 0"); | ||
if (angleOffset <= 0) | ||
throw new ArgumentException("angleOffset must be greater than 0"); | ||
|
||
this.angleOffset = angleOffset * Math.PI / 180; | ||
this.radius = radius; | ||
this.start = start; | ||
} | ||
|
||
public Point GetNextPoint() | ||
{ | ||
var nextPoint = GetPointByPolarCords(); | ||
angle += angleOffset; | ||
return nextPoint; | ||
} | ||
|
||
private Point GetPointByPolarCords() | ||
{ | ||
var offsetPerRadian = radius / (2 * Math.PI); | ||
var radiusVector = offsetPerRadian * angle; | ||
|
||
var x = (int)Math.Round(radiusVector * Math.Cos(angle) + start.X); | ||
var y = (int)Math.Round(radiusVector * Math.Sin(angle) + start.Y); | ||
|
||
return new Point(x, y); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<OutputType>Exe</OutputType> | ||
<TargetFramework>net8.0</TargetFramework> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="SkiaSharp" Version="3.118.0-preview.1.2" /> | ||
<PackageReference Include="SkiaSharp.Views.WindowsForms" Version="3.118.0-preview.1.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. А для чего превью-версии? Почему стабильную не можем использовать? Превью версии это фактически тестовый нюгет пакет, там могут быть уязвимости, итоговый пакет может отличаться абстракциями и тд 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. не смог задаунгрейдить, понял, в будущем будут инсталить стабильные стараться.
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. ааааа, короче там трабла там на ласт стабильной dotnet 6.0, такие дела |
||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,32 @@ | ||
using System.Drawing; | ||
using SkiaSharp; | ||
using SkiaSharp.Views.Desktop; | ||
|
||
|
||
namespace TagsCloudVisualization; | ||
|
||
public class Visualizer | ||
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. Визуалайзер чего? |
||
{ | ||
private readonly SKBitmap bitmap; | ||
private readonly SKCanvas canvas; | ||
|
||
public Visualizer(int width, int height) | ||
{ | ||
bitmap = new SKBitmap(width, height); | ||
canvas = new SKCanvas(bitmap); | ||
} | ||
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. 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. Не нужно |
||
|
||
public SKBitmap VisualizeTagCloud(IEnumerable<Rectangle> rectangles) | ||
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. Давай тогда везде использовать абстракции из SkiaSharp. А то странно же получается, что где-то System.Drawing, а где то SkiaSharp) Интерфейс изначальный давай тоже мб поменяем? 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. 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. Не уверен в чем тут проблема..... вроде размеры не очень большие типа 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 paint = new SKPaint | ||
{ | ||
Color = SKColors.White, | ||
Style = SKPaintStyle.Stroke | ||
}; | ||
|
||
foreach (var rectangle in rectangles) | ||
canvas.DrawRect(rectangle.ToSKRect(), paint); | ||
|
||
return bitmap; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,119 @@ | ||
using System.Drawing; | ||
using FluentAssertions; | ||
using NUnit.Framework.Interfaces; | ||
using TagsCloudVisualization; | ||
|
||
namespace TagsCloudVisualizationTests; | ||
|
||
[TestFixture] | ||
[TestOf(typeof(CircularCloudLayouter))] | ||
public class CircularCloudLayouterTest | ||
{ | ||
private const string ImagesDirectory = "../../../failedTests"; | ||
private readonly Random randomizer = new(); | ||
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. Опять же, рандом не потокобезопасный. Если появятся асинхронные тесты, то придется отлавливать отдельно |
||
private CircularCloudLayouter circularCloudLayouter; | ||
|
||
[SetUp] | ||
public void Setup() | ||
{ | ||
circularCloudLayouter = TestContext.CurrentContext.Test.Name.Contains("Optimal") ? | ||
CreateCircularCloudLayouterWithOptimalParams() : CreateCircularCloudLayouterWithRandomParams(); | ||
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. Вносим неявную конвенцию через ненадежный источник - имя теста. Так делать не стоит 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. Я хотел Category поюзать, но стало лень) |
||
} | ||
|
||
[TearDown] | ||
public void TearDown() | ||
{ | ||
var currentContext = TestContext.CurrentContext; | ||
if (currentContext.Result.Outcome.Status != TestStatus.Failed) return; | ||
|
||
var layoutSize = GetLayoutSize(circularCloudLayouter.Rectangles.ToList()); | ||
var visualizer = new Visualizer(layoutSize.Width, layoutSize.Height); | ||
var bitmap = visualizer.VisualizeTagCloud(circularCloudLayouter.Rectangles); | ||
|
||
var saver = new Saver(ImagesDirectory); | ||
var filename = $"{currentContext.Test.Name}.png"; | ||
saver.SaveAsPng(bitmap, filename); | ||
|
||
TestContext.Out.WriteLine($"Tag cloud visualization saved to file {Path.Combine(ImagesDirectory, filename)}"); | ||
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. TearDown : System.Exception : Unable to allocate pixels for the bitmap. 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. Странно у меня все тесты рисует... 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. на каком такое падает? 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. 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. |
||
} | ||
|
||
[Test] | ||
public void PutNextRectangle_ShouldReturnRectangle() | ||
{ | ||
var rectSize = randomizer.NextSize(1, int.MaxValue); | ||
|
||
var rect = circularCloudLayouter.PutNextRectangle(rectSize); | ||
|
||
rect.Should().BeOfType<Rectangle>(); | ||
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. Какойй смысл у этого теста? 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. убрал, смысл был проверить тип, что уже и так делает интерфейс |
||
} | ||
|
||
[Test] | ||
public void PutNextRectangle_ShouldReturnRectangleAtCenter_WhenFirstInvoked() | ||
{ | ||
var rectSize = randomizer.NextSize(1, int.MaxValue); | ||
|
||
var actualRect = circularCloudLayouter.PutNextRectangle(rectSize); | ||
var expectedRect = CircularCloudLayouter.CreateRectangle(circularCloudLayouter.Center, rectSize); | ||
|
||
actualRect.Should().BeEquivalentTo(expectedRect); | ||
} | ||
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. Почему мы тестируем один метод через то, что он вызывает внутри себя? Если в этом методе будет баг, то мы не отловим в тестах этого |
||
|
||
[Test] | ||
public void PutNextRectangle_ShouldReturnRectangle_WithCorrectSize() | ||
{ | ||
var recSize = randomizer.NextSize(1, int.MaxValue); | ||
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 actualRect = circularCloudLayouter.PutNextRectangle(recSize); | ||
|
||
actualRect.Size.Should().Be(recSize); | ||
} | ||
|
||
[Test] | ||
public void PutNextRectangle_ShouldReturnRectangles_WithoutIntersections() | ||
{ | ||
var numberOfRectangles = randomizer.Next(100, 300); | ||
|
||
var rectangles = Enumerable | ||
.Range(0, numberOfRectangles) | ||
.Select(_ => circularCloudLayouter.PutNextRectangle(randomizer.NextSize(10, 27))) | ||
.ToList(); | ||
|
||
rectangles.Any(fr => rectangles.Any(sr => fr != sr && fr.IntersectsWith(sr))) | ||
.Should().BeFalse(); | ||
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. Оч запутано, прям вчитываться приходится. Можем упростить? |
||
} | ||
|
||
[Test] | ||
public void GeneratedLayout_ShouldHaveHighTightnessAndShapeOfCircularCloud_WithOptimalParams() | ||
{ | ||
const double eps = 0.35; | ||
var rectangles = PutRandomRectanglesInLayouter(randomizer.Next(500, 1000)); | ||
var layoutSize = GetLayoutSize(rectangles); | ||
|
||
var diameterOfCircle = Math.Max(layoutSize.Width, layoutSize.Height); | ||
var areaOfCircle = Math.PI * Math.Pow(diameterOfCircle, 2) / 4; | ||
var areaOfRectangles = (double)rectangles | ||
.Select(r => r.Height * r.Width) | ||
.Sum(); | ||
var areaRatio = areaOfCircle / areaOfRectangles; | ||
|
||
areaRatio.Should().BeApproximately(1, eps); | ||
} | ||
|
||
private CircularCloudLayouter CreateCircularCloudLayouterWithOptimalParams() => new(new Point(0, 0)); | ||
private CircularCloudLayouter CreateCircularCloudLayouterWithRandomParams() => | ||
new(randomizer.NextPoint(-10, 10), randomizer.Next(1, 10), randomizer.Next(1, 10)); | ||
|
||
private List<Rectangle> PutRandomRectanglesInLayouter(int numberOfRectangles) => | ||
Enumerable | ||
.Range(0, numberOfRectangles) | ||
.Select(_ => circularCloudLayouter.PutNextRectangle(randomizer.NextSize(10, 27))) | ||
.ToList(); | ||
|
||
private Size GetLayoutSize(List<Rectangle> rectangles) | ||
{ | ||
var layoutWidth = rectangles.Max(r => r.Right) - rectangles.Min(r => r.Left); | ||
var layoutHeight = rectangles.Max(r => r.Top) - rectangles.Min(r => r.Bottom); | ||
|
||
return new Size(layoutWidth, layoutHeight); | ||
} | ||
} |
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.
Ага, то есть я могу сюда засунуть, например, рандомный генератор точек. Тогда круг из прямоугольников у нас не получится, как заявлено в названии
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.
поправил