-
Notifications
You must be signed in to change notification settings - Fork 305
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
Конина Анастасия #229
base: master
Are you sure you want to change the base?
Конина Анастасия #229
Changes from all commits
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 @@ | ||
global using NUnit.Framework; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<TargetFramework>net7.0</TargetFramework> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
|
||
<IsPackable>false</IsPackable> | ||
<IsTestProject>true</IsTestProject> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="FluentAssertions" Version="7.0.0-alpha.3" /> | ||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="17.6.0"/> | ||
<PackageReference Include="NUnit" Version="3.12.0"/> | ||
<PackageReference Include="NUnit3TestAdapter" Version="4.2.1"/> | ||
<PackageReference Include="NUnit.Analyzers" Version="3.6.1"/> | ||
<PackageReference Include="coverlet.collector" Version="6.0.0"/> | ||
</ItemGroup> | ||
|
||
<ItemGroup> | ||
<ProjectReference Include="..\TagsCloudVisualization\TagsCloudVisualization.csproj" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
namespace TagCloudVisualization.Tests; | ||
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. Имя файла обычно совпадает с именем класса. Сейчас имя файла |
||
using System; | ||
using System.Collections.Generic; | ||
using System.Drawing; | ||
using FluentAssertions; | ||
using System.Linq; | ||
using NUnit.Framework; | ||
using NUnit.Framework.Interfaces; | ||
using TagsCloudVisualization; | ||
|
||
[TestFixture] | ||
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 class CircularCloudLayouter_Should | ||
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 cloud; | ||
private Point center; | ||
private List<Rectangle> rectangles; | ||
|
||
[SetUp] | ||
public void Setup() | ||
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 Point(250, 250); | ||
cloud = new CircularCloudLayouter(center); | ||
rectangles = new List<Rectangle>(); | ||
} | ||
|
||
[TearDown] | ||
public void TearDown() | ||
{ | ||
if (TestContext.CurrentContext.Result.Outcome.Status != TestStatus.Failed) | ||
return; | ||
|
||
var path = TestContext.CurrentContext.TestDirectory + "\\" + TestContext.CurrentContext.Test.Name + ".bmp"; | ||
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. Интерполяция строк в помощь |
||
CloudDrawer.DrawAndSaveCloud(rectangles.ToArray(), path); | ||
TestContext.WriteLine("Tag cloud visualization saved to file: " + path); | ||
} | ||
|
||
[Test] | ||
public void PutNextRectangle_NoIntersects_AfterPutting() | ||
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. Нигде нет структуры AAA |
||
{ | ||
var rectangleSize = new Size(10, 50); | ||
|
||
rectangles = new List<Rectangle>(); | ||
for (var i = 0; i < 100; i++) | ||
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. Достаточно сложный тест сам по себе, хотя от этого и никуда не деться вроде как. Тем не менее, его можно чуть улучшить. |
||
{ | ||
rectangles.Add(cloud.PutNextRectangle(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. Все прямоугольники тут одинакового размера. Т.е. твое облако тегов в таком случае так же должно состоять из слов одинакового размера, что является пограничным случаем. К тому же у тебя все прямоугольники в виде столбцов Использовать |
||
} | ||
|
||
foreach (var rectangle in rectangles) | ||
{ | ||
foreach (var secondRectangle in rectangles.Where(x => x != rectangle)) | ||
{ | ||
rectangle.IntersectsWith(secondRectangle).Should().BeFalse(); | ||
} | ||
} | ||
} | ||
|
||
[Test] | ||
public void PutNextRectangle_OnCenter_AfterFirstPut() | ||
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 rectangleSize = new Size(100, 50); | ||
var expectedRectangle = new Rectangle(center, rectangleSize); | ||
var rectangle = cloud.PutNextRectangle(rectangleSize); | ||
rectangles.Add(rectangle); | ||
rectangle.Should().BeEquivalentTo(expectedRectangle); | ||
} | ||
|
||
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. Думаю, стоит добавить тестов на генератор точек. Например, я могу написать |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
using System.Drawing; | ||
using System.Collections.Generic; | ||
using System.Drawing.Imaging; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public class CloudDrawer | ||
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 static void DrawAndSaveCloud(Rectangle[] rectangles, string fileName) | ||
{ | ||
var bitmap = new Bitmap(500, 500); | ||
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 gr = Graphics.FromImage(bitmap); | ||
gr.FillRectangle(Brushes.Black, 0, 0, 500, 500); | ||
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. Кажется, если сделать не кисть, а карандаш, то твои прямоугольники буду полые, тогда на них будет проще смотреть. Если карандаш не поможет, то всё равно стоит поискать пути сделать только контуры прямоугольников для наглядности. |
||
gr.FillRectangles(Brushes.White, rectangles); | ||
bitmap.Save(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. Что будет, если вывалится |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public interface IPointsGenerator | ||
{ | ||
IEnumerable<Point> GetPoints(); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public interface ITagCloudLayouter | ||
{ | ||
Rectangle PutNextRectangle(Size rectangleSize); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
using System.Drawing; | ||
using System.Collections.Generic; | ||
using TagsCloudVisualization; | ||
|
||
public class CircularCloudLayouter : ITagCloudLayouter | ||
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 Point cloudCenter; | ||
private readonly IList<Rectangle> _rectangels = new List<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.
|
||
private readonly SpiralPointsGenerator spiralPointsGenerator; | ||
|
||
|
||
public CircularCloudLayouter(Point cloudCenter) | ||
{ | ||
this.cloudCenter = cloudCenter; | ||
spiralPointsGenerator = new SpiralPointsGenerator(cloudCenter); | ||
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 Rectangle PutNextRectangle(Size rectangleSize) | ||
{ | ||
foreach (var point in spiralPointsGenerator.GetPoints()) | ||
{ | ||
var rectangle = new Rectangle(point, rectangleSize); | ||
if (_rectangels.Any(x => x.IntersectsWith(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. Переделывать не надо, но есть ощущение (смотря на твои изображения), что генератор не отдает предпочтения горизонтальному расположение прямоугольников - хотя большая часть слов располагаться будет все же горизонтально. |
||
continue; | ||
|
||
_rectangels.Add(rectangle); | ||
return 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. Перед |
||
} | ||
|
||
return new Rectangle(cloudCenter, rectangleSize); | ||
} | ||
|
||
static void Main(string[] args) | ||
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 @@ | ||
100 квадратов со случайными размерами (максимальный размер - 70) | ||
![TagCloud](100(randomButLesser70)Rectangles.png ) | ||
|
||
40 квадратов со случайными размерами (максимальный размер - 50) | ||
![TagCloud](40(randomButLesser50)Rectangles.png ) | ||
|
||
100 квадратов со сторонами 10, 50 | ||
![TagCloud](100(10,50)Rectangles.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. Хотелось бы сразу увидеть большее кол-во - 500 / 1000, например. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,35 @@ | ||
using System.Drawing; | ||
|
||
namespace TagsCloudVisualization; | ||
|
||
public sealed class SpiralPointsGenerator : IPointsGenerator | ||
{ | ||
private readonly Point center; | ||
private readonly double radiusStep; | ||
|
||
public SpiralPointsGenerator(Point center, double radiusStep = 1) | ||
{ | ||
this.center = center; | ||
this.radiusStep = radiusStep; | ||
} | ||
|
||
///<exception cref="System.OverflowExeption"> | ||
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. Почти, но не совсем. Описание метода должно быть отдельно - в |
||
/// Long point generation operation | ||
/// </exception> | ||
public IEnumerable<Point> GetPoints() | ||
{ | ||
double radius = 0; | ||
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 term = radiusStep / 360; | ||
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. Что такое |
||
while (true) | ||
{ | ||
for (var pointNumber = 0; pointNumber < 360; pointNumber++) | ||
{ | ||
var pointAngle = 2 * Math.PI * pointNumber / 360; | ||
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 currentPoint = new Point(center.X, center.Y); | ||
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.
Как максимум вынести это в отдельный метод, дав ему название того, что он действительно делает - переводит из полярной системы координат в декартову. |
||
currentPoint.Offset((int)(Math.Cos(pointAngle) * radius), (int)(Math.Sin(pointAngle) * radius)); | ||
yield return currentPoint; | ||
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. Стоит добавить отступов, чтоб ясно видеть |
||
radius += term; | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
<Project Sdk="Microsoft.NET.Sdk"> | ||
|
||
<PropertyGroup> | ||
<OutputType>Exe</OutputType> | ||
<TargetFramework>net7.0</TargetFramework> | ||
<ImplicitUsings>enable</ImplicitUsings> | ||
<Nullable>enable</Nullable> | ||
<GenerateProgramFile>false</GenerateProgramFile> | ||
</PropertyGroup> | ||
|
||
<ItemGroup> | ||
<PackageReference Include="FluentAssertions" Version="6.12.0" /> | ||
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. Мы с тобой вынесли тесты в отдельную папку и отдельный проект. Следовательно, из этого проекта нужно убрать ненужные зависимости. |
||
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.7.1" /> | ||
<PackageReference Include="NUnit" Version="3.12.0" /> | ||
<PackageReference Include="NUnit3TestAdapter" Version="3.17.0" /> | ||
<PackageReference Include="System.Drawing.Common" Version="8.0.0" /> | ||
</ItemGroup> | ||
|
||
</Project> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"sdk": { | ||
"version": "8.0.0", | ||
"rollForward": "latestMajor", | ||
"allowPrerelease": true | ||
} | ||
} |
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.
Старайся использовать надежные версии, без всяких альфа/бета и прочих наворотов.