Вильданов Савелий #241

Saveliy21 · 2024-11-12T18:48:00Z

desolver · 2024-11-14T18:49:18Z

cs/TagsCloudVisualization/CircularCloudLayouter.cs

+    public Rectangle PutNextRectangle(Size rectangleSize)
+    {
+        Rectangle rectangle;
+        if (rectangleSize.Width < 0 || rectangleSize.Height < 0)


А есть ли смысл работать с нулевой шириной или высотой?

desolver · 2024-11-14T18:49:50Z

cs/TagsCloudVisualization/Spiral.cs

+        angle = 0;
+    }
+
+    public Point getNextPointOnSpiral()


Публичный метод с маленькой буквы

desolver · 2024-11-14T18:51:41Z

cs/TagsCloudVisualization/CircularCloudLayouter.cs

+    private List<Rectangle> rectangles;
+    private Spiral spiral;


Эти поля тоже можно readonly сделать

desolver · 2024-11-14T18:57:20Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/CircularCloudLayotherTest.cs

+    [Test]
+    public void PutNextRectangle_ShouldAddNewRectangle()
+    {
+        CircularCloudLayouter circularCloudLayouter = new CircularCloudLayouter(new Point());


Создание этого класса происходит во всех тестах этого класса - можно вынести в SetUp, как обязательное действие перед прогоном теста. Плюсом будет то, что код самих тестов станет читать попроще

desolver · 2024-11-14T18:58:05Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/CircularCloudLayotherTest.cs

+        CircularCloudLayouter circularCloudLayouter = new CircularCloudLayouter(new Point());
+        circularCloudLayouter.PutNextRectangle(new Size(2, 2));
+        circularCloudLayouter.GetRectangles().Should().Contain(new Rectangle(-1, -1, 2, 2));


По выполняемым в тесте действиям этот тест идентичен предыдущему, а значит и проверку из этого теста можно включить в предыдущий

desolver · 2024-11-14T19:00:29Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/CircularCloudLayotherTest.cs

+    }
+
+    [Test]
+    public void PutNextRectangle_ShouldHaveCorrectSize()


Из названия теста не совсем понятно, Size чего будет проверяться в тесте - размещенного прямоугольника или коллекции с размещенными прямоугольниками

desolver · 2024-11-14T19:01:32Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/CircularCloudLayotherTest.cs

+public class CircularCloudLayotherTest
+{
+    [TestCase(-2, 3, TestName = "Negative width")]
+    [TestCase(-2, 3, TestName = "Negative height")]


В Negative height тесте высота положительная)

desolver · 2024-11-14T19:04:00Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/SpiralTest.cs

+    [Test]
+    public void getNextPointOnSpiral_ShouldReturnNextPointOnSpiral()


Давай тест на несколько размещаемых точек ещё напишем

desolver · 2024-11-14T19:08:59Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/CircularCloudLayotherTest.cs

+
+namespace TagsCloudVisualization.TagsCloudVisualizationTests;
+
+public class CircularCloudLayotherTest


Было бы круто ещё написать тест на вставку ооооочень большого количества прямоугольников, проверим, что он отработает за некоторое вменяемое время

Проверок внутри не нужно будет, у Nunit есть атрибут, который завершит тест, если тот не уложится в заданное время

desolver · 2024-11-14T19:13:50Z

cs/TagsCloudVisualization/TagsCloudVisualizationTests/CircularCloudLayotherTest.cs

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

Давай и тут выделим под тесты отдельный проект

desolver · 2024-11-19T17:12:41Z

cs/TagsCloudVisualization/CloudGenerator.cs

+        Bitmap bmp = new Bitmap(1000, 1000);
+        Graphics newGraphics = Graphics.FromImage(bmp);
+        var backgroundBrush = new SolidBrush(Color.White);
+        var rectanglePen = new Pen(Color.Black);


Все ли ресурсы тут освобождены после того, как они стали не нужными?

desolver · 2024-11-19T17:14:19Z

cs/TagsCloudVisualization/CloudGenerator.cs

+
+namespace TagsCloudVisualization;
+
+public static class CloudGenerator


Этот класс взвалил на себя две ответственности - он и прямоугольники генерирует, и в файл сохраняет, а это все же разные ответственности. Давай сохранение в файл вынесем в отдельный модуль. А метод GenerateRectangles просто запишем в Program.cs как обычный вызывающий код

desolver · 2024-11-19T17:16:39Z

cs/TagsCloudVisualizationTests/CloudGeneratorTest.cs

+
+namespace TagsCloudVisualization.TagsCloudVisualizationTests;
+
+public class CloudGeneratorTest


Этот класс тестировать не обязательно (именно ту часть, которая просто вызывает лейаутер и его построение), так как это не основной код твоей программы. Он просто вызывает основные модули и методы в определенном порядке. А после вынесения в Program.cs и тестировать будет нечего))

А вот тест на сохранение в файл тест можно будет оставить

desolver · 2024-11-19T17:17:50Z

cs/TagsCloudVisualizationTests/CloudGeneratorTest.cs

+        CircularCloudLayouter circularCloudLayouter =
+            new CircularCloudLayouter(new Point(CloudLayouterConst.CloudCentreX, CloudLayouterConst.CloudCentreY));
+        circularCloudLayouter.PutNextRectangle(new Size(2, 2));


Это лишняя работа в тесте. Когда тестируем сохраняльщик в файл, то нам не важно, откуда взялись прямоугольники, нам важно просто их нарисовать. Поэтому можешь тут просто руками создать список, лейаутер вызывать не нужно

desolver · 2024-11-19T17:18:35Z

cs/TagsCloudVisualizationTests/TagsCloudVisualizationTests.csproj

+    </PropertyGroup>
+
+    <ItemGroup>
+      <PackageReference Include="FluentAssertions" Version="7.0.0-alpha.5" />


альфа пакеты на свой страх и риск))

desolver · 2024-11-21T17:29:47Z

cs/TagsCloudVisualization/CloudDrawer.cs

+        Bitmap bmp = new Bitmap(1000, 1000);
+        Graphics newGraphics = Graphics.FromImage(bmp);
+        var backgroundBrush = new SolidBrush(Color.White);
+        var rectanglePen = new Pen(Color.Black);


Все ещё не освободил ресурсы тут

using var bmp = new Bitmap(1000, 1000); using var newGraphics = Graphics.FromImage(bmp); using var backgroundBrush = new SolidBrush(Color.White); using var rectanglePen = new Pen(Color.Black);

Классы, которые реализуют IDisposable можно с помощью синтакисческого сахара диспоузить через using, когда они завершат выполнение в своем скоупе

desolver · 2024-11-21T17:30:50Z

cs/TagsCloudVisualization/CloudDrawer.cs

+{
+    public static Bitmap DrawRectangles(List<Rectangle> rectangles)
+    {
+        Bitmap bmp = new Bitmap(1000, 1000);


Если прямоугольников будет много, они могут просто не влезть в этот размер, почему именно 1000 на 1000?

desolver · 2024-11-21T17:31:25Z

cs/TagsCloudVisualization/CloudDrawer.cs

+            newGraphics.DrawRectangle(rectanglePen, rectangle);
+        }
+
+        newGraphics.Dispose();


Это можно убрать, юзинга будет достаточно

desolver · 2024-11-21T17:33:26Z

cs/TagsCloudVisualization/TagsCloudVisualization.csproj

@@ -9,6 +9,7 @@
    </PropertyGroup>

    <ItemGroup>
+      <PackageReference Include="FluentAssertions" Version="6.12.2" />


А зачем пакет для ассертов в проекте, где он не будет использоваться?

desolver · 2024-11-21T17:34:03Z

cs/TagsCloudVisualizationTests/TagsCloudVisualizationTests.csproj

@@ -5,10 +5,11 @@
        <TargetFramework>net8.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
+        <RootNamespace>TagsCloudVisualisationTests</RootNamespace>


Можно не добавлять, по умолчанию RootNamespace равен названию проекта

desolver · 2024-11-21T17:35:54Z

cs/TagsCloudVisualizationTests/CloudDrawerTest.cs

+
+namespace TagsCloudVisualization.TagsCloudVisualizationTests;
+
+public class CloudDrawerTest


Это название уже не актуально, по факту внутри тестируется не CloudDrawer

desolver · 2024-11-21T17:36:46Z

cs/TagsCloudVisualization/SaveImages.cs

+
+namespace TagsCloudVisualization;
+
+public class SaveImages


Классы называем существительными, сейчас у тебя получился глагол "Сохрани изображения", а надо "Сохранятель изображений"))

Saveliy21 added 6 commits November 12, 2024 23:32

create interface

dee9824

Add CircularCloudLayotherTest

a1d1e33

add CircularCloudLayouter

1fb56fb

Add SpiralTest

16fb3d6

Add class Spiral

42f7bf7

add settings

f86d62f

Saveliy21 changed the title ~~Home work~~ Вильданов Савелий Nov 13, 2024

desolver suggested changes Nov 14, 2024

View reviewed changes

Saveliy21 added 16 commits November 15, 2024 17:05

refactor CircularCloudLayouter.cs

ef1be52

refactor Spiral.cs

3415138

refactor CircularCloudLayotherTest.cs

38ceb83

refactor SpiralTest.cs

4de5007

refactor project Tests

d20083c

change Spiral settings

198e49c

create CloudGenerator.cs

069d48f

create Program.cs

492c16e

Add Images

ec49a56

small refactor Program.cs

597425e

change Images location

4dc529e

create class for constants

a1aab21

refactor CloudGenerator.cs

42d0b33

Add TearDown

bbb7ffa

refactor SpiralTest.cs

27c162d

Add CloudGeneratorTest.cs

5e7cbd6

desolver suggested changes Nov 19, 2024

View reviewed changes

Saveliy21 added 5 commits November 21, 2024 13:58

Add new class SaveImages

fdc168c

change FluentAssertions version

801bba9

refactor CircularCloudLayotherTest.cs

2ecfb96

replace CloudGenerator.cs to CloudDrawer.cs

0119e8d

replace CloudGeneratorTest to CloudDrawerTest

1a2d746

refactor Program.cs

083d09f

desolver suggested changes Nov 21, 2024

View reviewed changes

Saveliy21 added 11 commits November 22, 2024 17:12

add new method to CircularCloudLayouter.cs

0a052f3

refactor CloudDrawer.cs

b4d4356

rename class

71c304c

refactor Program.cs

a045a06

refactor TagsCloudVisualizationTests.csproj

12d47d1

refactor TagsCloudVisualization.csproj

f16502f

rename CloudDrawerTest.cs

1097f79

refactor CircularCloudLayotherTest.cs

0ac7a7c

unit SaveImage.cs with CloudDrawer.cs

fb56b45

refactor

d625800

small refactor

3ce2afc

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Вильданов Савелий #241

Вильданов Савелий #241

Saveliy21 commented Nov 12, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 14, 2024

desolver Nov 19, 2024

desolver Nov 19, 2024

desolver Nov 19, 2024

desolver Nov 19, 2024

desolver Nov 19, 2024

desolver Nov 21, 2024

desolver Nov 21, 2024

desolver Nov 21, 2024

desolver Nov 21, 2024

desolver Nov 21, 2024

desolver Nov 21, 2024

desolver Nov 21, 2024

		[Test]
		public void getNextPointOnSpiral_ShouldReturnNextPointOnSpiral()


		namespace TagsCloudVisualization.TagsCloudVisualizationTests;

		public class CircularCloudLayotherTest


		namespace TagsCloudVisualization;

		public static class CloudGenerator


		namespace TagsCloudVisualization.TagsCloudVisualizationTests;

		public class CloudGeneratorTest


		namespace TagsCloudVisualization.TagsCloudVisualizationTests;

		public class CloudDrawerTest

Вильданов Савелий #241

Are you sure you want to change the base?

Вильданов Савелий #241

Conversation

Saveliy21 commented Nov 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment