Сазонов Александр #237

AlexxSaz · 2024-11-12T14:57:59Z

Изначально начал пилить кастомные классы, но потом обнаружил библиотеку System.Drawing. Все коммиты с кастомными классами оставил в отдельной ветке

cs/TagsCloudVisualization/TagsCloudVisualization.sln

cs/TagsCloudVisualization/TagsCloudVisualization/CircularCloudLayouter.cs

cs/TagsCloudVisualization/TagsCloudVisualization/CircularCloudLayouterShould.cs

cs/TagsCloudVisualization/TagsCloudVisualization/PointExtensionShould.cs

cs/TagsCloudVisualization/TagsCloudVisualization/PointGenerator.cs

dmitgaranin · 2024-11-12T17:14:55Z

cs/TagsCloudVisualization/TagsCloudVisualization/PointGeneratorShould.cs

+        for (var i = 1; i <= stepCount; i++)
+        {
+            var currPoint = _defaultPointGenerator.GetNewPoint();
+            if (i % stepToCheck == 0)


А почему именно 100 шаг? Пока не понимаю, что проверяет этот тест в таких органичениях

Я все еще не понимаю, зачем конструкция if (i % radiusCheckPeriod != 0) continue; )

cs/TagsCloudVisualization/ICircularCloudLayouter.cs

cs/TagsCloudVisualization/TagsCloudVisualization/CircularCloudLayouterShould.cs

cs/TagsCloudVisualization/CircularCloudLayouterShould.cs

dmitgaranin · 2024-11-14T07:38:02Z

cs/TagsCloudVisualization/CircularCloudLayouterShould.cs

+        var width = maxX - minX;
+        var height = maxY - minY;
+
+        return width / height;


А можешь рассказать логику этого теста? Как он проверяет, что итоговые прямогольники похожи на круг

Там была идея, что круг может быть вписан в квадрат, а у квадрата соотношение сторон единица. Тест переписал на другую логику

dmitgaranin · 2024-11-14T07:38:53Z

cs/TagsCloudVisualization/CircularCloudLayouterShould.cs

+
+        var circleSquare = Math.PI * radius * radius;
+        var density = fullSquare / circleSquare;
+        density.Should().BeGreaterThan(0.4);


И про логику этого теста интересно послушать)

Убрал этот тест и написал новый с другой логикой=)

cs/TagsCloudVisualization/PointGenerator.cs

cs/TagsCloudVisualization/TagsCloudVisualization/PointGenerator.cs

cs/TagsCloudVisualization/PointGeneratorShould.cs

dmitgaranin · 2024-11-14T07:42:04Z

cs/TagsCloudVisualization/TagsCloudVisualization/PointGeneratorShould.cs

+        for (var i = 1; i <= stepCount; i++)
+        {
+            var currPoint = _defaultPointGenerator.GetNewPoint();
+            if (i % stepToCheck == 0)


Я все еще не понимаю, зачем конструкция if (i % radiusCheckPeriod != 0) continue; )

cs/TagsCloudVisualization/CircularCloudLayouter.cs

dmitgaranin · 2024-11-17T12:35:20Z

cs/TagsCloudVisualization/TagCloudVisualization.cs

+    {
+        const int rectangleOutline = 1;
+
+        var bitmap = new Bitmap(


Почему-то почти все строчки в этом файле подсвечиваются

Замечание не поправлено

Атрибут - решение, но костыльное
Правильное решение <TargetFramework>net8.0-windows</TargetFramework>, т.к. если есть виндо-зависимый код, то стоит собирать его явно

dmitgaranin · 2024-11-17T12:38:10Z

cs/TagsCloudVisualization/TagCloudVisualization.cs

+
+    private static Color GetRandomColor()
+    {
+        var knownColors = (KnownColor[])Enum.GetValues(typeof(KnownColor));


Можно ли как-нибудь сократить количество вычисления этого постоянного числа knownColors?

Исправление корректное
А как сократить в случае старого кода?)

Вынести за пределы метода, сделать статичным полем класса

dmitgaranin · 2024-11-17T12:40:09Z

cs/TagsCloudVisualization/TagCloud.cs

+
+namespace TagsCloudVisualization;
+
+public class TagCloud(ICloudLayouter? layouter)


Выглядит слегка как еще одна реализация ICloudLayouter. А может и нет

Этот класс служит по факту для хранения размеров лейаута любого вида (если он реализует ICloudLayouter)

cs/TagsCloudTests/CloudLayouterShould.cs

dmitgaranin · 2024-11-17T12:54:33Z

cs/TagsCloudTests/CloudLayouterShould.cs

+    {
+        var largestSide = _random.Next(100, 200);
+        var rectangleSizes = GetSizes(_random.Next(5, 10), largestSide);
+        var radii = new List<double>();


Что такое radii?

гугл сказал, что это множественное число от radius=)

dmitgaranin · 2024-11-17T12:57:27Z

cs/TagsCloudTests/CloudLayouterShould.cs

@@ -0,0 +1,121 @@
+using FluentAssertions;


Не нашел тестов на :

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

Переписал тесты. В новых коммитах:
тест PutNextRectangle_ReturnRectanglesInCircle_AfterManyExecution проверяет, что сумма площади всех прямоугольников примерно равна площади окружности, в которой прямоугольники находятся. А также, что расстояние от каждого прямоугольника до центра лейаута меньше, чем радиус окружности. Этот тест говорит о том, что форма облака - окружность, а также, что облако максимально плотное.

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

dmitgaranin · 2024-11-17T12:59:23Z

cs/TagsCloudTests/CloudLayouterShould.cs

+namespace TagsCloudTests;
+
+[TestFixture]
+[Parallelizable(scope: ParallelScope.All)]


А можно ли этот атрибут применить на всю сборку? Чтобы на каждый класс не вешать его

да, если использовать наследование, где базовый класс будет иметь все необходимые нам атрибуты

А еще варианты? Можем в шарпе есть решение?

Можно через таски. Условно сделать массив тасок со всеми тестами и запустить их параллельно

Можно через атрибут на всю сборку - [assembly: Parallelizable(ParallelScope.Children)]

Атрибут - ок) Также могу порекомендовать такие глобальные штуки выносить в самостоятельные файлы, т.к. они не принадлежат конкретному файлу с тестами, а всем)

cs/TagsCloudTests/TagCloudCreatorShould.cs

…pure now

dmitgaranin · 2024-11-18T11:51:01Z

cs/TagsCloudTests/CloudLayouterShould.cs

+        var circleSquare = circleRadius * circleRadius * Math.PI;
+        var precision = circleSquare * 0.475;
+
+        circleSquare.Should().BeApproximately(sumRectanglesSquare, precision);


Для множественного should нужно использовать враппинг AssertionScope, чтобы проверял все-все. Потому что если один из них упадет, тест закончится

А еще можно опустить вложенность) и будет читать вообще легко

resolved

dmitgaranin · 2024-11-18T11:52:07Z

cs/TagsCloudTests/CloudLayouterShould.cs

+
+        circleSquare.Should().BeApproximately(sumRectanglesSquare, precision);
+        foreach (var distanceToCenter in fromRectangleToCenterDistances)
+            distanceToCenter.Should().BeLessOrEqualTo(circleRadius + 2);


Почему + 2? Может ли тест перестать работать без единого исправления в нем?

Проверил. Тест падал примерно один раз на несколько тысяч.

Переделал расчет радиуса окружности, теперь нету +2. И увеличил нижнюю планку в рандоме количества прямоугольников, что помогло снизить коэффициент аппроксимации. Тест больше не падает.

cs/TagsCloudTests/PointExtensionShould.cs

cs/TagsCloudTests/PointGeneratorShould.cs

cs/TagsCloudTests/TagsCloudTests.csproj

AlexxSaz added 9 commits November 9, 2024 11:41

Initial TagsCloudVisualization commit

ad2d818

Custom classes raplaced by system drawing lib

e68c53d

Inited points generator class

f5e166b

Added implementation to GetNewPoint and tests

55e34ba

PointGenerator refactored

6d20961

Implementation changed to use PointGenerator

8056773

Refactored CircularCLoudLayouter

d667276

Created extention method for Point

9e1ec6f

Point generator refactored

898414c

dmitgaranin reviewed Nov 12, 2024

View reviewed changes

AlexxSaz added 6 commits November 13, 2024 12:29

Solution changed to tdd.sln

7e04cfb

Point extension removed

903e69b

Tests moved to special namespace

86704ff

Point generator tests refactored

d0636b7

Interface created

891e73d

Added new tests

8fdf444

dmitgaranin reviewed Nov 14, 2024

View reviewed changes

cs/TagsCloudVisualization/ICircularCloudLayouter.cs Outdated Show resolved Hide resolved

cs/TagsCloudVisualization/TagsCloudVisualization/CircularCloudLayouterShould.cs Outdated Show resolved Hide resolved

dmitgaranin reviewed Nov 14, 2024

View reviewed changes

AlexxSaz added 11 commits November 15, 2024 20:02

Created new project for tests

1320a4a

Created new interface

91a0388

Point generator tests moved to new project

fd798c6

Cloud layouter test moved and renamed

53157f1

TagCloud added

9047c0e

Cherry picked extension for point

e71f57f

Added extension for rectangle

0ac9619

Changed point extensions

3ad22ea

Fixed getting new rectangle in CircularCloudLayouter

3b25de2

TagCloud refactored

18aa897

PointGenerator refactored

0e7de68

AlexxSaz added 12 commits November 16, 2024 23:35

Some tests refactored

3843478

Added new tests to tag cloud

e42563c

Added test to check that layouter has circular form

055b888

Added class that creates tag cloud example

02a9b74

Installation of default objects has been changed

574bcc8

Angle step changed

f983fe4

Reworked layouter tests for new angle step

692ce6c

Added visualization class

677cdf9

Created samples and readme

5f6471e

GenereatePoint method reworked to pure method

0647fae

Added teardown to save failed pictures

99b191c

Refactored layouter tests

7ee7523

dmitgaranin reviewed Nov 17, 2024

View reviewed changes

AlexxSaz added 8 commits November 17, 2024 21:38

CloudLayouter tests reworked

a777a93

Test replaced by program.cs

2b4a1ed

TagCloud properties reworked to constant time. But addRectangle isnt …

72851ba

…pure now

Replaced sample to random rectangles

a1d4e86

PointGenerator test for spiral is reworked

af5b600

Some things refactored

1acce94

Rework method that returns new color

936f24c

Added new class to generate sizes

20fbb7f

dmitgaranin reviewed Nov 18, 2024

View reviewed changes

AlexxSaz added 6 commits November 19, 2024 17:17

Fluent style changed

ceda81c

CloudLayouter test fixed

8482ee9

Parallelizable accepted to whole project

a18455d

Removed virtual method

19ea2dc

Tag cloud properties reworked to expression body

25352da

Fixed drawing lib problem

57390e8

dmitgaranin mentioned this pull request Nov 27, 2024

Сазонов Александр kontur-courses/clean-code#233

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Сазонов Александр #237

Сазонов Александр #237

AlexxSaz commented Nov 12, 2024

dmitgaranin Nov 12, 2024

dmitgaranin Nov 14, 2024

dmitgaranin Nov 14, 2024

AlexxSaz Nov 17, 2024

dmitgaranin Nov 14, 2024

AlexxSaz Nov 17, 2024

dmitgaranin Nov 14, 2024

dmitgaranin Nov 17, 2024

dmitgaranin Nov 18, 2024

dmitgaranin Nov 19, 2024

dmitgaranin Nov 17, 2024

dmitgaranin Nov 18, 2024

AlexxSaz Nov 19, 2024

dmitgaranin Nov 17, 2024

AlexxSaz Nov 18, 2024

dmitgaranin Nov 17, 2024

AlexxSaz Nov 17, 2024

dmitgaranin Nov 17, 2024

AlexxSaz Nov 18, 2024

dmitgaranin Nov 17, 2024

AlexxSaz Nov 17, 2024

dmitgaranin Nov 18, 2024

AlexxSaz Nov 18, 2024

dmitgaranin Nov 18, 2024

dmitgaranin Nov 19, 2024

dmitgaranin Nov 18, 2024

dmitgaranin Nov 19, 2024

dmitgaranin Nov 18, 2024

AlexxSaz Nov 19, 2024


		namespace TagsCloudVisualization;

		public class TagCloud(ICloudLayouter? layouter)

Сазонов Александр #237

Are you sure you want to change the base?

Сазонов Александр #237

Conversation

AlexxSaz 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

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