Попов Захар #201

BlizPerfect · 2025-01-16T15:09:58Z

Примеры работы программы и результаты выполнения прошлых частей задания по ссылкам:

1 часть: Попов Захар tdd#257
2 часть: Попов Захар di-updated#18

FontParser теперь возвращает FontFamily

1. Добавлена фабрика по созданию WordFilter. 2. Добавлена фабрика по созданиюCloudLayouterWorker. 3. Рефакторинг DIContainer.

1. wordsToIncludeFile - добавление слов в фильтр "скучных слов" из файла. 2. wordsToExcludeFile- исключение слов из фильтра "скучных слов" из файла. 3. normalizeMinCoefficient - минимальный коэффициент нормализации. 3. Рефакторинг WordFilter.

…agCloud

1. Рефакторинг CloudLayouterPainter. 2. Добавлен новый тест.

1. Рефакторинг ImageSaver. 2. Обновление тестов.

1. Рефакторинг Normalizer. 2. Обновление тестов.

1. Рефакторинг ColorParser. 2. Добавлены тесты для ColorParser.

1. Рефакторинг FontParser. 2. Добавлены тесты для FontParser.

1. Рефакторинг SizeParser. 2. Добавлены тесты для SizeParser.

masssha1308 · 2025-01-18T08:28:18Z

FileSenderRailway/Result.cs


+[assembly: InternalsVisibleTo("TagCloud")]


А для чего? Заметила что это нужно для обращения к Value класса Result. Но ведь это поле не просто так было сделано internal :) Например, вот здесь есть проверка что при вычислении ширины и высоты не было ошибок

Так почему бы вместо прямого доступа к Value не воспользоваться GetValueOrThrow?

Это только для того, что бы класс Result.cs был виден для моих проектов. Можно было перекопировать этот класс в сам проект, но я решил просто сделать его видимым.

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

Это только для того, что бы класс Result.cs был виден для моих проектов. Можно было перекопировать этот класс в сам проект, но я решил просто сделать его видимым.

Не совсем понятно, почему бы публичный класс из другого проекта не будет виден без этого атрибута?)

masssha1308 · 2025-01-18T08:30:12Z

TagCloud.Tests/CloudLayouterPaintersTest/CloudLayouterPainterTest.cs

+
+namespace TagCloud.Tests.CloudLayouterPaintersTest
+{
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Проверка совместимости платформы", Justification = "<Ожидание>")]


System.Diagnostics.CodeAnalysis.SuppressMessage длинновато, давай сократим с помощью юзинга

А вообще, для чего этот атрибут здесь и что значит "ожидание"?

Это подавление сообщения об ограниченности использования класса Bitmap:

Можно было сделать так:

#pragma warning disable CA1416 var dummyImage = new Bitmap(1, 1); #pragma warning restore CA1416

Изначально добавлял игнорирование этой ошибки в GlobalSuppressions.cs:

// This file is used by Code Analysis to maintain SuppressMessage // attributes that are applied to this project. // Project-level suppressions either have no target or are given // a specific target and scoped to a namespace, type, member, etc. using System.Diagnostics.CodeAnalysis; [assembly: SuppressMessage("Interoperability", "CA1416:Проверка совместимости платформы", Justification = "<Ожидание>", Scope = "member", Target = "~M:TagCloud.Tests.ImageSaversTests.ImageSaverTests.SaveFile_ThrowsException_WithInvalidFormat(System.String)")]

Justification обозначает вроде как причину, почему это подавление добавлено. Студия по умолчания туда прописывает "<Ожидание>" - ожидается. что когда-то там исправят. Я понимаю, что это плохая практика и в продакшене так конечно не надо делать.

ожидается. что когда-то там исправят

Ожидается) Так может и поправим?)

masssha1308 · 2025-01-18T10:32:44Z

TagCloud.Tests/CloudLayouterPaintersTest/CloudLayouterPainterTest.cs

+namespace TagCloud.Tests.CloudLayouterPaintersTest
+{
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Проверка совместимости платформы", Justification = "<Ожидание>")]
+    internal class CloudLayouterPainterTest


Кажется тут все тесты с одинаковым смыслом - невалидный параметр на входе и проверяется что выкидывается исключение с нужным текстом. Давай объедим все это дело в один тест с testcases. Тогда и setUp можно будет убрать. И еще не хватает проверки позитивных сценариев. Не только что все "как надо" ломается, а еще и что работает правильно

masssha1308 · 2025-01-18T10:39:36Z

...s/CloudLayouterTests/CircularCloudLayouterTests/CircularCloudLayouterMainRequirementsTest.cs

+{
+    [TestFixture]
+    [System.Diagnostics.CodeAnalysis.SuppressMessage("Interoperability", "CA1416:Проверка совместимости платформы", Justification = "<Ожидание>")]
+    internal class CircularCloudLayouterMainRequirementsTest


Давай объединим CircularCloudLayouterMainRequirementsTest и CircularCloudLayouterTest. Не вижу смысла в выделении отдельного класса для теста с exception. + смущает "MainRequirements" в названии, я бы тоже убрала (если не согласен, то опиши зачем оно)

Я согласен, что конкретно сейчас название "CircularCloudLayouterMainRequirementsTest" не котируется, но оно появилось из первой части задания, как выделение проверки основным требованиям:

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

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

Люди которые в дальнейшем работали бы с кодом этого приложения не знали бы об этих требованиях и этом ТЗ. Поэтому для них логика в названии бы отсутствовала. Нужно поправить

Из работы с прошлым наставником, я лично подчерпнул, что надо не бояться создавать множество тестовых файлов

Нужно исправить т.к. "основные" - понятие непонятное :) хранители знания что такое в данном контексте "основное" - только люди которые видели ТЗ к задаче. А у человека который видит этот код впервые это вызовет недоумение. Следовательно, это мешает читабельности кода

А так, да. Создавать тестовые файлы не нужно бояться если есть на то причина. Очень советую почитать 9 главу "Микросевисы" Ричардсона

masssha1308 · 2025-01-18T10:42:25Z

...s/CloudLayouterTests/CircularCloudLayouterTests/CircularCloudLayouterMainRequirementsTest.cs

+
+namespace TagCloud.Tests.CloudLayouterTests.CircularCloudLayouterTests
+{
+    [TestFixture]


Где-то есть TestFixture, а где-то нет. Хочется придерживаться единого стиля. А вообще, в используемой версии nunit TestFixture необязателен

masssha1308 · 2025-01-18T11:14:58Z

...s/CloudLayouterTests/CircularCloudLayouterTests/CircularCloudLayouterMainRequirementsTest.cs

+            var maxRectangleWidth = 70;
+            var minRectangleHeight = 20;
+            var maxRectangleHeight = 50;
+            var rectanglesCount = 1000;


почему именно такие значения? может быть их тоже лучше рандомайзером генерить?

На мой взгляд, рандомить границы, для того, что бы затем рандомить размер внутри этих границ - оверкил в данном случае, так как в этом тесте мы проверяем соответствие основным требованиям из первой части задания для ICloudLayouter - форма, плотность, отсутствие пересечений прямоугольников. А эти значения используются для вспомогательного класса, которые и выдаёт размеры прямоугольников, которые затем будут расставлены.

masssha1308 · 2025-01-18T11:21:35Z

...s/CloudLayouterTests/CircularCloudLayouterTests/CircularCloudLayouterMainRequirementsTest.cs

+            rectangles = tags.Select(x => x.Rectangle).ToArray();
+        }
+
+        [TestCase(0.7, 1000)]


Есть ли смысл выносить testCase когда он один?

masssha1308 · 2025-01-18T11:25:10Z

...s/CloudLayouterTests/CircularCloudLayouterTests/CircularCloudLayouterMainRequirementsTest.cs

+            }
+        }
+
+        private (int start, int end) GetGridIndexesInterval(


Хорошо что вынес в отдельные методы, так намного читабельнее

masssha1308 · 2025-01-18T11:43:40Z

TagCloud.Tests/ImageSaversTests/ImageSaverTest.cs

+            Directory.CreateDirectory(directoryPath);
+        }
+
+        [SetUp]


а точно ли нужен такой setUp? В чем преимущество перед тем чтобы вместо imageSaver в тестах писать new ImageSaver()?

masssha1308 · 2025-01-18T11:45:31Z

TagCloud.Tests/MainTest.cs

+namespace TagCloud.Tests
+{
+    [TestFixture]
+    internal class MainTest() : BaseOptionTest("MainTest")


Неинформативное название

masssha1308 · 2025-01-18T11:48:09Z

TagCloud.Tests/NormalizersTest/NormalizerTest.cs

+namespace TagCloud.Tests.NormalizersTest
+{
+    [TestFixture]
+    internal class NormalizerTest


В некоторых классах в названии не tests, а test. Давай все такие места поправим на tests

masssha1308 · 2025-01-18T11:55:00Z

TagCloud.Tests/OptionsTests/BackgroundColorTests.cs

+namespace TagCloud.Tests.OptionsTests
+{
+    [TestFixture]
+    internal class BackgroundColorTests() : BaseOptionTest("BackgroundColor")


А зачем выделять для этого теста отдельный класс, почему его нельзя поместить в MainTest? Кажется по смыслу он туда подходит. Классы стоит выделать по функциональности которую проверяем (в данном случае это тест на ProgramExecutor) поэтому на мой взгляд нет необходимости проверку фона при работе выносить в отдельный класс. Если класс получится слишком большим и будет напрашиваться разделение, можно выделить 2: негативные и позитивные сценарии

Тот же самый вопрос к классам TextColorTest, FontTest и т.д.

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

Название папки - OptionsTests - тестируются аргументы командной строки, передаваемые нашей программе.

Название файла/Класса - FontTests - тестируются опции, относящиеся к шрифтам, которые мы передаём как аргументы для работы нашей программы.

Название теста - Program_WorksCorrectly_WithInvalidFont - тестируется работа программы, при передаче через командную строку некорректного аргумента, относящегося к шрифтам.

Из работы с прошлым наставником я такую логику структурирования подчерпнул.

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

masssha1308 · 2025-01-18T12:03:16Z

TagCloud.Tests/OptionsTests/ValidValues.cs

+{
+    internal static class ValidValues
+    {
+        public readonly static string[] ValidDataFileContent = new string[]


inconsistent modifiers declaration order

masssha1308 · 2025-01-18T12:07:48Z

TagCloud.Tests/WordCountersTests/WordCounterTest.cs

+                { "Three", 1 },
+                { "Four", 4 },
+            };
+            var values = new string[]


Кажется такое есть в ValidValues

masssha1308 · 2025-01-18T12:11:32Z

fp.sln

@@ -1,14 +1,21 @@

 Microsoft Visual Studio Solution File, Format Version 12.00


кажется в mr попали лишние файлы

…атформах.

…ости

… метод GetValueOrThrow()

1. Тесты с CircularCloudLayouterMainRequirementsTest добавлены в CircularCloudLayouterTest. 2. Рефакторинг CircularCloudLayouterTest.

BlizPerfect added 19 commits January 10, 2025 19:42

Добавлены проекты из TagCloudll

195211f

Старые классы, выбрасывающие исключения, переведены на паттерн Result<T>

741d7f6

Рефакторинг кода FontParser.

ee91491

FontParser теперь возвращает FontFamily

Общий рефакторинг кода по результатам проверки второй части задания T…

aaa88e2

…agCloud

Рефакторинг CloudLayouterPainter.

0d4319b

1. Рефакторинг CloudLayouterPainter. 2. Добавлен новый тест.

Рефакторинг CircularCloudLayouter.

5a8ab71

Рефакторинг NormalizedFrequencyBasedCloudLayouterWorker.

a3f9304

Рефакторинг RandomCloudLayouterWorker.

76f46ac

Рефакторинг ImageSaver.

14d48e8

1. Рефакторинг ImageSaver. 2. Обновление тестов.

Рефакторинг Normalizer.

a518eec

1. Рефакторинг Normalizer. 2. Обновление тестов.

Добавлены тесты для BoolParser.

5381142

Рефакторинг ColorParser.

80b703a

1. Рефакторинг ColorParser. 2. Добавлены тесты для ColorParser.

Рефакторинг FontParser.

437d5c4

1. Рефакторинг FontParser. 2. Добавлены тесты для FontParser.

Рефакторинг SizeParser.

741ea65

1. Рефакторинг SizeParser. 2. Добавлены тесты для SizeParser.

Рефакторинг TagCloud.

98b047f

Добавлены тесты для проверка корректности переданных аргументов.

0235996

Рефакторинг тестов.

3d8eeec

masssha1308 reviewed Jan 18, 2025

View reviewed changes

BlizPerfect added 13 commits January 20, 2025 22:09

Рефакторинг сообщения об ограниченности поддержки Bitmap на разных пл…

dd9a5cd

…атформах.

Рефакторинг теста для CloudLayouterPainter

c35158a

Во всех тестовых классах добавлен недостающий атрибут [TestFixture]

000025c

Атрибут [TestCase] у тестов с одним случаем исправлен на просто [Test]

866b9c9

Рефакторинг тестов с избыточным атрибутом [SetUp]

eaa377b

MainTest переименован в FullProgramExecution, для большей информативн…

2f02a57

…ости

Исправлен порядок модификаторов

51d00af

Исправелно дублирование кода в WordCounterTest

0a9a7c6

Названия тестов приведены к единообразию

01cc083

Добавлено 2 новых теста

1139482

Замена использования internal свойства Value у Result<T> на публичный…

9557b19

… метод GetValueOrThrow()

Добавлено сообщение о ограниченности выполнения кода

42110bb

Рефакторинг CircularCloudLayouterTest.

b2fbe3b

1. Тесты с CircularCloudLayouterMainRequirementsTest добавлены в CircularCloudLayouterTest. 2. Рефакторинг CircularCloudLayouterTest.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Попов Захар #201

Попов Захар #201

BlizPerfect commented Jan 16, 2025 •

edited

Loading

masssha1308 Jan 18, 2025

BlizPerfect Jan 20, 2025 •

edited

Loading

masssha1308 Jan 21, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

BlizPerfect Jan 20, 2025 •

edited

Loading

masssha1308 Jan 21, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

BlizPerfect Jan 20, 2025

masssha1308 Jan 21, 2025 •

edited

Loading

masssha1308 Jan 21, 2025

masssha1308 Jan 21, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

BlizPerfect Jan 20, 2025 •

edited

Loading

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025 •

edited

Loading

masssha1308 Jan 18, 2025

BlizPerfect Jan 20, 2025 •

edited

Loading

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

masssha1308 Jan 18, 2025

		@@ -1,14 +1,21 @@

		Microsoft Visual Studio Solution File, Format Version 12.00

Попов Захар #201

Are you sure you want to change the base?

Попов Захар #201

Conversation

BlizPerfect commented Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

BlizPerfect Jan 20, 2025 • edited Loading

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

BlizPerfect Jan 20, 2025 • edited Loading

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

masssha1308 Jan 21, 2025 • edited Loading

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

BlizPerfect Jan 20, 2025 • edited Loading

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

masssha1308 Jan 18, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BlizPerfect Jan 20, 2025 • edited Loading

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

BlizPerfect commented Jan 16, 2025 •

edited

Loading

BlizPerfect Jan 20, 2025 •

edited

Loading

BlizPerfect Jan 20, 2025 •

edited

Loading

masssha1308 Jan 21, 2025 •

edited

Loading

BlizPerfect Jan 20, 2025 •

edited

Loading

masssha1308 Jan 18, 2025 •

edited

Loading

BlizPerfect Jan 20, 2025 •

edited

Loading