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

Брайденбихер Виктор Николаевич #238

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -197,4 +197,5 @@ FakesAssemblies/

*Solved.cs

.idea/
.idea/
/cs/tdd.sln.DotSettings
2 changes: 1 addition & 1 deletion cs/BowlingGame/BowlingGame.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
<PackageReference Include="FireSharp" Version="2.0.4" />
<PackageReference Include="FluentAssertions" Version="5.10.3" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" />
<PackageReference Include="Newtonsoft.Json" Version="12.0.3" />
<PackageReference Include="Newtonsoft.Json" Version="13.0.3" />
<PackageReference Include="NUnit" Version="3.12.0" />
<PackageReference Include="NUnit.Console" Version="3.11.1" />
<PackageReference Include="NUnit3TestAdapter" Version="3.17.0" />

Choose a reason for hiding this comment

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

Старайся чтобы в мр попадало только то, что действительно нужно. Скорее всего проект BowlingGame вы рассматривали во время занятия, но к домашке он никак не относится.

Expand Down
51 changes: 51 additions & 0 deletions cs/TagsCloudVisualization/CloudClasses/CircularCloudLayouter.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
using System.Drawing;
using TagsCloudVisualization.CloudClasses.Interfaces;

namespace TagsCloudVisualization.CloudClasses
{
public class CircularCloudLayouter : ICloudLayouter
{
public readonly List<Rectangle> Rectangles;

public readonly Point Center;

public readonly IRayMover RayMover;

public CircularCloudLayouter(Point center, double radiusStep = 1, double angleStep = 5)
{
if (center.X <= 0 || center.Y <= 0)
throw new ArgumentException("Center Point should have positive X and Y");

if (radiusStep <= 0 || angleStep <= 0)
throw new ArgumentException("radiusStep and angleStep should be positive");

Rectangles = new List<Rectangle>();

Choose a reason for hiding this comment

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

Не обязательно выносить создание коллекций в конструктор класса — их можно сразу инициализировать прямо в полях.

Center = center;

RayMover = new SpiralMover(center, radiusStep, angleStep);
Copy link

@ksamnole ksamnole Nov 16, 2024

Choose a reason for hiding this comment

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

Давай будем передавать в конструктор IRayMover. Чтобы можно было легко их подменять, если вдруг появятся другие реализации IRayMover помимо SpiralMover

}

public Rectangle PutNextRectangle(Size rectangleSize)
{
if (rectangleSize.Width <= 0 || rectangleSize.Height <= 0)
throw new ArgumentException("Not valid size should be positive");

Choose a reason for hiding this comment

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

Текст ошибки недостаточно понятен, лучше как то переформулировать)


foreach (var point in RayMover.MoveRay())
{
var location = new Point(point.X - rectangleSize.Width / 2,
point.Y - rectangleSize.Height / 2);

var rectangle = new Rectangle(location, rectangleSize);

// Проверяем, пересекается ли новый прямоугольник с уже существующими
if (!Rectangles.Any(r => r.IntersectsWith(rectangle)))
{
Rectangles.Add(rectangle);
return rectangle;
}
}

return new Rectangle();

Choose a reason for hiding this comment

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

Как то нехорошо выглядит, что мы можем в какой-то теоретической ситуации вернуть Rectangle без параметров, подумай как можно исключить эту ситуацию.

}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Drawing;

namespace TagsCloudVisualization.CloudClasses.Interfaces
{
public interface ICloudLayouter
{
public Rectangle PutNextRectangle(Size rectangleSize);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
using System.Drawing;

namespace TagsCloudVisualization.CloudClasses.Interfaces
{
public interface IRayMover
{
IEnumerable<Point> MoveRay();
}
}
35 changes: 35 additions & 0 deletions cs/TagsCloudVisualization/CloudClasses/Ray.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
using System.Drawing;

namespace TagsCloudVisualization.CloudClasses
{
public class Ray
{
public double Radius { get; private set; }

//В радианах
public double Angle { get; private set; }

public readonly Point RayStart;

Choose a reason for hiding this comment

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

nit: Контекст класса уже говорит о том что поле относится к классу Ray, необязательно его называть RayStart. Лучше что-то типа StartPosition или StartPoint


public Ray(Point rayStart, int radius, int angle)
{
RayStart = rayStart;

Radius = radius;

Angle = angle;
}

public void Update(double deltaRadius, double deltaAngle)
{
Radius += deltaRadius;
Angle += deltaAngle;
}

public Point GetEndPoint => new Point

Choose a reason for hiding this comment

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

nit: лишнее слово Get

{
X = RayStart.X + (int)(Radius * Math.Cos(Angle)),
Y = RayStart.Y + (int)(Radius * Math.Sin(Angle))
};
}
}
38 changes: 38 additions & 0 deletions cs/TagsCloudVisualization/CloudClasses/SpiralMover.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
using System.Drawing;
using TagsCloudVisualization.CloudClasses.Interfaces;

namespace TagsCloudVisualization.CloudClasses
{
public class SpiralMover : IRayMover
{
public readonly Ray SpiralRay;

public readonly double RadiusStep;

//В радианах
public readonly double AngleStep;

public const double OneRound = Math.PI * 2;

public SpiralMover(Point center, double radiusStep = 1, double angleStep = 5,
int startRadius = 0, int startAngle = 0)
{
SpiralRay = new Ray(center, startRadius, startAngle);
RadiusStep = radiusStep;

//Преобразование из градусов в радианы
AngleStep = angleStep * Math.PI / 180;
}

public IEnumerable<Point> MoveRay()
{
while (true)
{
yield return SpiralRay.GetEndPoint;

//Радиус увеличивается на 1 только после полного прохождения круга
SpiralRay.Update(RadiusStep / OneRound * AngleStep, AngleStep);
}
}
}
}
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.
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.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
32 changes: 32 additions & 0 deletions cs/TagsCloudVisualization/TagsCloudVisualization.csproj
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
<Project Sdk="Microsoft.NET.Sdk">

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

<IsPackable>false</IsPackable>
<IsTestProject>true</IsTestProject>
<StartupObject>TagsCloudVisualization.Visualisation.Program</StartupObject>
</PropertyGroup>

<ItemGroup>
<PackageReference Include="coverlet.collector" Version="6.0.0" />
<PackageReference Include="FluentAssertions" Version="6.12.2" />
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.8.0" />
<PackageReference Include="NUnit" Version="3.14.0" />
<PackageReference Include="NUnit.Analyzers" Version="3.9.0" />
<PackageReference Include="NUnit3TestAdapter" Version="4.5.0" />
<PackageReference Include="System.Drawing.Common" Version="8.0.10" />
</ItemGroup>

<ItemGroup>
<Using Include="NUnit.Framework" />
</ItemGroup>

<ItemGroup>
<Folder Include="CorrectImages\" />
<Folder Include="FailImages\" />
</ItemGroup>

</Project>
185 changes: 185 additions & 0 deletions cs/TagsCloudVisualization/Tests/CircularCloudLayouterTests.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
using FluentAssertions;
using NUnit.Framework.Interfaces;
using System.Drawing;
using TagsCloudVisualization.CloudClasses;
using TagsCloudVisualization.Visualisation;

namespace TagsCloudVisualization.Tests

Choose a reason for hiding this comment

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

Я бы разделил на два проекта: текущий, где будет находится вся логика и проект где будут тесты на логику.
Это сделает код более понятным и удобным - логика остается чистой, а тесты находятся отдельно и не мешают.

Choose a reason for hiding this comment

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

Плюс артефакты из тестов, будут складываться рядом с тестами, а не посреди логики

Copy link
Author

Choose a reason for hiding this comment

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

Я первоначально так и хотел, но когда я прочитал формулировку задания в репозитории
"Сделай форк этого репозитория. Добавь проект TagsCloudVisualization. Выполняй задания в этом проекте."
Это заставило меня передумать и создать один проект, но я понимаю, что лучше было бы это разделить

Choose a reason for hiding this comment

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

Понял, немного от формулировки задания, можно отходить если считаешь как можно сделать лучше

{
public class Tests

Choose a reason for hiding this comment

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

Название класса не совпадает с названием файла

{
private const int WIDTH = 1000;

private const int HEIGHT = 1000;

private CircularCloudLayouter _layouter;

private Point _center;

private IVisualiser _visualiser;

private Bitmap _currentBitmap = null;

private TagCloudImageGenerator _tagCloudImageGenerator;

[SetUp]
public void Setup()
{
_center = new Point(WIDTH / 2, HEIGHT / 2);
_layouter = new CircularCloudLayouter(_center);

_visualiser = new Visualiser(Color.Black, 1);
_tagCloudImageGenerator = new TagCloudImageGenerator(_visualiser);
}

[TearDown]
public void Teardown()
{
if (TestContext.CurrentContext.Result.Outcome == ResultState.Failure && _currentBitmap != null)

Choose a reason for hiding this comment

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

Чтобы не делать проверку на null, можно создавать bitmap прямо в этом методе и не хранить его отдельно

{
var fileName = $"{TestContext.CurrentContext.Test.MethodName + Guid.NewGuid()}.png";

BitmapSaver.SaveToFail(_currentBitmap, fileName);
_currentBitmap.Dispose();
}

_visualiser.Dispose();
_tagCloudImageGenerator.Dispose();
}

[Test]
public void CircularCloudLayouter_WhenCreated_RectanglesShoudBeEmpty()
{
_layouter.Rectangles.Should().BeEmpty();
}

[Test]
public void CircularCloudLayouter_WhenCreated_FirstPointEqualsLayouterCenter()
{
var firstPoint = _layouter.RayMover.MoveRay().First();

firstPoint.Should().BeEquivalentTo(_layouter.Center);
}

[Test]
public void CircularCloudLayouter_WhenAddFirstRectangle_ContainOneAndSameRectangle()
{
var rectangle = _layouter.PutNextRectangle(new Size(20, 10));

_layouter.Rectangles
.Select(r => r).Should().HaveCount(1).And.Contain(rectangle);
}

[Test]
public void CircularCloudLayouter_WhenAddRectangle_ReturnsRectangleWithSameSize()
{
var givenSize = new Size(20, 20);

var rectangle = _layouter.PutNextRectangle(givenSize);

rectangle.Size.Should().BeEquivalentTo(givenSize);
}

[TestCase(0, 0)]
[TestCase(-2, 2)]
[TestCase(2, -2)]
[TestCase(-2, -2)]
public void CircularCloudLayouter_WhenWrongSize_ThrowArgumentException(int width, int height)
{
Assert.Throws<ArgumentException>(() => _layouter.PutNextRectangle(new Size(width, height)),
"Not valid size should be positive");
}

[TestCase(0, 0)]
[TestCase(-2, 2)]
[TestCase(2, -2)]
[TestCase(-2, -2)]
public void CircularCloudLayouter_WhenCenterWrong_ThrowArgumentException(int x, int y)
{
Assert.Throws<ArgumentException>(() => new CircularCloudLayouter(new Point(x, y)),
"Center Point should have positive X and Y");
}

[TestCase(0, 0)]
[TestCase(-2, 2)]
[TestCase(2, -2)]
[TestCase(-2, -2)]
public void CircularCloudLayouter_WhenNotValidSteps_ThrowArgumentException(int radiusStep, int angleSter)
{
Assert.Throws<ArgumentException>(() => new CircularCloudLayouter(new Point(10, 10), radiusStep, angleSter),
"radiusStep and angleStep should be positive");
}

[TestCase(5)]

Choose a reason for hiding this comment

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

Можно заменить просто на Test, раз уж тест всего один

public void CircularCloudLayouter_WhenAddFew_ShouldHaveSameCount(int add)
{
_currentBitmap = _tagCloudImageGenerator.CreateNewBitmap(new Size(WIDTH, HEIGHT),
_layouter,
() =>
{
return SizeBuilder.Configure()
.SetCount(add)
.SetWidth(60, 80)
.SetHeight(60, 80)
.Generate();
});

_layouter.Rectangles.Should().HaveCount(add);
}

[TestCase(1, 2, 3)]
public void CircularCloudLayouter_WhenAddFewButWasBefore_ShouldHaveCorrectCount(int countBefore, int add, int countAfter)

Choose a reason for hiding this comment

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

Честно скажу не понимаю что значит "WhenAddFewButWasBefore"

{
//������� ��������� countBefore
_currentBitmap = _tagCloudImageGenerator.CreateNewBitmap(new Size(WIDTH, HEIGHT),
_layouter,
() =>
{
return SizeBuilder.Configure()
.SetCount(countBefore)
.SetWidth(60, 80)
.SetHeight(60, 80)
.Generate();
});

//������� ��������� countAfter
_tagCloudImageGenerator.AddToCurrentImage(_currentBitmap,
_layouter,
() =>
{
return SizeBuilder.Configure()
.SetCount(add)
.SetWidth(60, 80)
.SetHeight(60, 80)
.Generate();
});

_layouter.Rectangles.Should().HaveCount(countAfter);
}

[TestCase(2)]
[TestCase(30)]
public void CircularCloudLayouter_WhenAddFew_RectangleNotIntersectsWithOtherRectangles(int count)
{
//������� ��������� countBefore
_currentBitmap = _tagCloudImageGenerator.CreateNewBitmap(new Size(WIDTH, HEIGHT),
_layouter,
() =>
{
return SizeBuilder.Configure()
.SetCount(count)
.SetWidth(60, 80)
.SetHeight(60, 80)
.Generate();
});

foreach (var rectangle in _layouter.Rectangles)
{
_layouter.Rectangles
//�� �������� ��������� ������ ����.....
.Any(r => r.IntersectsWith(rectangle) && rectangle != r)
.Should().BeFalse();
}
}
}
}
Loading