Муканов Арман #203

MNYOU · 2024-01-25T10:22:58Z

No description provided.

razor2651 · 2024-01-25T13:48:00Z

TagsCloudContainer/CloudLayouters/CircularCloudLayouter.cs

+    private const double FullCircle = Math.PI * 2;
+    private const int SpiralStepThreshold = 10;
+
+    public CircularCloudLayouter(ImageSettings imageSettings)


давай будем зависеть от абстракций а не от конкретики

razor2651 · 2024-01-25T13:48:14Z

TagsCloudContainer/CloudLayouters/CircularCloudLayouter.cs

+        spiralStep = 1;
+    }
+
+    public IReadOnlyList<Rectangle> AddedRectangles => rectangles;


лишнее

razor2651 · 2024-01-25T13:53:42Z

TagsCloudContainer/Settings/CloudData.cs

@@ -0,0 +1,8 @@
+namespace TagsCloudContainer.Settings;
+
+public class CloudData


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

razor2651 · 2024-01-25T13:58:25Z

TagsCloudContainer/TextAnalysers/TextPreprocessor.cs

+            .Select(inf => inf.Split('=').First().ToLower());
+        UpdateCloud(words);
+
+        return cloudData;


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

razor2651 · 2024-01-25T13:58:53Z

TagsCloudContainer/TextAnalysers/TextPreprocessor.cs

+    private readonly MyStem myStem;
+    private readonly AnalyseSettings settings;
+
+    public TextPreprocessor(CloudData cloudData, MyStem myStem, AnalyseSettings settings)


используй абстракции

razor2651 · 2024-01-25T15:20:04Z

TagsCloudContainer/ITagsCloudGenerator.cs

+
+public interface ITagsCloudGenerator
+{
+    public IEnumerable<Tag> Generate();


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

razor2651 · 2024-01-25T15:22:09Z

ConsoleApp/ConsoleApp.csproj

@@ -0,0 +1,18 @@
+<Project Sdk="Microsoft.NET.Sdk">


хорошо что выделил в отдельную сборку CUI

razor2651 · 2024-01-25T15:22:54Z

TagsCloudContainer.Tests/TagsCloudGeneratorTests.cs

@@ -0,0 +1,15 @@
+namespace TagsCloudContainer.Tests;
+
+public class TagsCloudGeneratorTests


у тебя же должны были остаться тесты после предыдущего блока с генератором облака, почему хотя бы те тесты не перенес сюда?

razor2651 · 2024-01-25T15:26:39Z

ConsoleApp/Program.cs

+
+internal class Program
+{
+    public static void Main(string[] args)


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

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

razor2651 · 2024-01-25T15:30:07Z

ConsoleApp/CommandLineParsers/Handlers/IOptionsHandler.cs

+
+public interface IOptionsHandler<in TOptions> : IOptionsHandler where TOptions : IOptions
+{
+    public void Map(TOptions options);


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

чем это все отличается от вот такого интерфейса? (сейчас все интерфейсы у тебя по сути можно заменить на этот)

public inteface IDoEverything { void Do(); }

razor2651 · 2024-01-31T13:32:29Z

TagsCloudContainer/CloudGenerators/TagsCloudGenerator.cs

+public class TagsCloudGenerator: ITagsCloudGenerator
+{
+    private readonly ITagTextMeasurer tagTextProvider;
+    private ICloudLayouterProvider cloudLayouterProvider;


razor2651 · 2024-01-31T13:33:54Z

TagsCloudContainer/AnalyzeData.cs

@@ -0,0 +1,6 @@
+namespace TagsCloudContainer;
+
+public class AnalyzeData


что инкапсулирует такой класс?

Analyze - глагол, такое название подошло бы методу а не названию контракта

razor2651 · 2024-01-31T13:39:42Z

TagsCloudContainer/WordData.cs

+        Frequency = frequency;
+    }
+
+    public static bool CanMap(string wordInfo)


почему ответственность за парсинг данных из утилиты mystem лежит здесь а не в препроцессоре?

razor2651 · 2024-01-31T13:50:09Z

TagsCloudContainer/TextAnalysers/TextPreprocessor.cs

+
+namespace TagsCloudContainer.TextAnalysers;
+
+public class TextPreprocessor: ITextPreprocessor


этот класс надо декомпозировать

занимается фильтрацией частей речи из MyStem

убирает скучные слова

подсчитывает частоты

что если мы захотим добавить еще фильтрацию не по частям речи а еще, например, по количеству букв, по первым буквам слов? в этом случае придется изменять поведение этого класса

razor2651 · 2024-01-31T13:54:58Z

TagsCloudContainer/TextAnalysers/TextPreprocessor.cs

+        if (string.IsNullOrWhiteSpace(wordInfo))
+            return true;
+
+        var data = wordInfo.Split('=');


там split, здесь split. еще в worddata split причем тех же самых данных, нарушаешь DRY

razor2651 · 2024-01-31T13:58:06Z

TagsCloudContainer/Settings/AppSettings.cs

+
+public class AppSettings: IAppSettings
+{
+    public string InputFile { get; set; } = @"C:\Users\mukan\Documents\Учёба\ShPORA\di\TagsCloudContainer\Data\Input.txt";


а на чужом компе будет работать?
и если оно все равно перезапишется то зачем тут эти значения

razor2651 · 2024-01-31T14:00:05Z

TagsCloudContainer/TagsCloudContainer.cs

+
+public class TagsCloudContainer: ITagsCloudContainer
+{
+    private readonly FileReader fileReader;


юзай абстракции

razor2651 · 2024-01-31T14:09:56Z

ConsoleApp/Program.cs

+        commandLineReader.ParseFromConsole();
+    }
+
+    public static void ConfigureService(ContainerBuilder builder)


обычно все таки регистрация зависиомстей происходит не ручками, представь что у тебя сотни классов, надо будет каждый зарегать и еще убедиться что никто не пропущен, для этого у autofac существует отдельный метод RegisterAllAssemblyTypes, но для этого надо еще правильно уметь правильно указывать сборки для регистрации, а вот там где нужно что то хитрое, какая то особая регистрация - вот в тех только местах и регистрировать явно

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

razor2651 · 2024-01-31T14:16:38Z

TagsCloudContainer/FileProviders/FileReader.cs

+{
+    public string ReadFile(string filePath)
+    {
+        return File.ReadAllText(filePath);


что если файла не будет/доступа нет/несуществующий путь итд

razor2651 · 2024-01-31T14:19:44Z

ConsoleApp/Handlers/IOptionsHandler.cs

+
+public interface IOptionsHandler
+{
+    public bool CanParse(object options);


почему тут object а не IOptions?

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

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

MNYOU added 3 commits January 25, 2024 15:17

feat: создал генератор облака тегов

79fdcc4

test: добавил тесты.

fe33b62

feat: добавил консольный клиент

4cfd0e7

razor2651 reviewed Jan 25, 2024

View reviewed changes

MNYOU added 3 commits January 31, 2024 15:10

docs: добавил примеры генерации

938d4dd

fix: исправил навзвание директории

21df153

refactor: рефакторинг

9add474

razor2651 reviewed Jan 31, 2024

View reviewed changes

MNYOU added 3 commits February 1, 2024 00:01

refactor: Декомпозировал TextPreprocessor.cs

9aa937e

refactor: переделал регистрацию на assembly

731ed78

fix: Переделал object на IOptions

4efacea

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Муканов Арман #203

Муканов Арман #203

MNYOU commented Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 25, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

razor2651 Jan 31, 2024

		@@ -0,0 +1,8 @@
		namespace TagsCloudContainer.Settings;

		public class CloudData

		@@ -0,0 +1,15 @@
		namespace TagsCloudContainer.Tests;

		public class TagsCloudGeneratorTests

		@@ -0,0 +1,6 @@
		namespace TagsCloudContainer;

		public class AnalyzeData


		namespace TagsCloudContainer.TextAnalysers;

		public class TextPreprocessor: ITextPreprocessor

Муканов Арман #203

Are you sure you want to change the base?

Муканов Арман #203

Conversation

MNYOU commented Jan 25, 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