Бессараб Дмитрий #31

d2em0n · 2024-12-25T12:28:52Z

@glazz200

GlazProject · 2024-12-26T16:16:13Z

TagsCloudContainer/TextProcessor/ITextProcessor.cs

+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;


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

GlazProject · 2024-12-26T16:17:53Z

TagsCloudContainer/TextProcessor/ITextProcessor.cs

+{
+    public interface ITextProcessor
+    {
+        public Dictionary<Word, int> Words(string path, ITextProvider provider, params IWordFilter[] filters);


Кажется фильтры - это всё-таки информация для конструктора, а не конкретного метода. Почему выбрал делать именно так?

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

GlazProject · 2024-12-26T16:20:44Z

TagsCloudContainer/TextProcessor/TextProcessor.cs

+            var words = new Dictionary<Word, int>();
+            foreach (var word in GetWordsFromString(provider.ReadFile(path)))
+            {
+                var flag = true;


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

GlazProject · 2024-12-26T16:29:34Z

TagsCloudContainer/TextProcessor/TextProcessor.cs

+                foreach (var filter in filters)
+                {
+                    if (flag == false) break;
+                    if (filter.Skips(word)) continue;
+                    flag = false;
+                }
+                if (flag == false) continue;


Как сложно получилось 😨
Есть предложение реализовать здесь паттерн фильтра (немного отличается от привычного значения) и упростить этот код. А так же дать возможность легко расширять набор этих предусловий
Вот здесьможно почитать про паттерн фильтра. Нашёл хорошую статью только для Java, но она очень похожа на C#. Если будут вопросы, разберём

GlazProject · 2024-12-26T16:30:31Z

TagsCloudContainer/TextProcessor/TextProcessor.cs

+                    flag = false;
+                }
+                if (flag == false) continue;
+                if (!words.TryGetValue(word, out var _))


Здесь лучше подойдёт ContainsKey по значению

А ещё есть практика делать универсальный код с небольшими подстановками. Вот что имею в виду

// Подготовка данных, на случай если ещё нет ничего if (!words.ContainsKey(word)) words[word] = 0; // Универсальное поведение для всех слов вне зависимости от происходящего words[word] += 1;

GlazProject · 2024-12-26T16:37:35Z

TagsCloudContainer/WordFilters/SimpleFilter.cs

+
+namespace TagsCloudContainer.WordFilters
+{
+    public class SimpleFilter : IWordFilter


Может тогда BoringWordFilter?

GlazProject · 2024-12-26T16:38:23Z

TagsCloudContainer/WordFilters/SimpleFilter.cs

+{
+    public class SimpleFilter : IWordFilter
+    {
+        private readonly HashSet<string> _words =


Дадим более узкое название этому списку исключений? А то только по операциям над ним становится понятно, что это такое

GlazProject · 2024-12-26T16:41:14Z

TagsCloudContainer/Tag.cs

+        Font = font;
+        Color = color;
+        var rect = g.MeasureString(Value, Font).ToSize();
+        Frame = new Size(rect.Width + 2, rect.Height + 2);


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

Кстати, размеры можно считать "снаружи" и передавать уже готовый size. Тогда это станет record

GlazProject · 2024-12-26T16:43:09Z

TagsCloudContainer/Program.cs

+            var provider = new TxtTextProvider();
+            var filter = new SimpleFilter();
+            var processor = new TextProcessor.TextProcessor();


Комментарий на будущее. DI контейнер заменит это на красивый блок кода, где не нужно будет задумываться над тем, как кого создать

GlazProject · 2024-12-26T16:46:49Z

TagsCloudContainer.Tests/TxtTextProviderShould.cs

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


Не хватает в названии условия, при котором бросать исключение

GlazProject · 2025-01-03T19:50:20Z

TagsCloudContainer/Configuration/Config.cs

+    public bool RandomColor;
+    public Color Color { get; set; }


Названия у этих свойств не очень отражают суть. Получится придать им смысла? И ещё не очень понятно, как быть в ситуации, когда RandomColor=true и цвет тоже указан. Можно посмотреть в сторону nullable типов

GlazProject · 2025-01-03T19:51:29Z

TagsCloudContainer/Configuration/DependencyInjection.cs

+        var container = new ContainerBuilder();
+
+        container.RegisterType(config.PointGenerator)
+            .AsImplementedInterfaces()


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

GlazProject · 2025-01-03T19:52:15Z

TagsCloudContainer/Configuration/DependencyInjection.cs

+                .AsImplementedInterfaces()
+                .SingleInstance();
+        else
+        {


Фигурные скобки сильно путают. Если у if их нет, то скорее всего в else тоже они не нужны

GlazProject · 2025-01-03T19:53:56Z

TagsCloudContainer/PointGenerators/ArchemedianSpiral.cs

+
+namespace TagsCloudContainer.PointGenerators
+{
+    [Label("Спираль")]


Это интересная техника, хорошо что прочитал. Но здесь на мой взгляд это перебор. Если что-то подобное делать, то нужно использовать локализации, а это уже совсем другая история. Предлагаю остановиться просто на названиях классов или даже цифрах при выборе. Это уже будет определяться реализацией клиента (ui или console)

GlazProject · 2025-01-03T19:56:10Z

TagsCloudContainer/TagGenerator/RandomColorTagGenerator.cs

+
+namespace TagsCloudContainer.TagGenerator
+{
+    public class RandomColorTagGenerator(ITextProcessor processor, Graphics graphics, Font defaultFont) : ITagsGenerator


Конструктор по умолчанию class Foo(prop1, prop2, prop3) может быть только у классов без методов (например record). Дело в том, что все эти поля создаются не readonly, что повышает риск ошибки

GlazProject · 2025-01-03T20:02:18Z

TagsCloudContainer/Program.cs

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


Если код ниже - эксперименты - хорошо. Но в итоговом решении его стоит вынести в отдельную реализацию клиента, и в Main вызывать уже его запуск

GlazProject · 2025-01-03T20:08:08Z

TagsCloudContainer/Program.cs

+
+        }
+
+        private static void ConfigureColor(Config config)


А вот вручную вызывать Assembly не нужно. Это ровно противоположное действие использованию DI контейнера. Задать возможные цвета через консоль поможет одна из библиотек парсинга аргументов. Например такой. Это заметно упростит обработку и поддержку кода. Чем более строгая типизация - тем лучше. Думаю, можно прямо на этапе разработки добавить нужные значения в аргументы. Они редко будут меняться. А что самое главное, при их изменении IDE подскажет, где есть использование, что позволит явно проконтролировать обратную совместимость (новая версия не ломает старое использование)

К тому же всё это теряет смысл в момент парсинга цвета, который производится вручную через switch

GlazProject · 2025-01-03T20:11:34Z

TagsCloudContainer/Program.cs

+            var pointGenerators = FindImplemetations<IPointGenerator>();
+            foreach (var point in pointGenerators)
+                Console.WriteLine(point.Key);
+            Console.WriteLine("Введите, соблюдая орфографию");


Аналогично предыдущему. Если хочется дать такую гибкую возможность конфигурирования, то можно воспользоваться функцией di возвращать набор зарегистрированных реализаций (container.Resolve<IEnumerable<IPointGenerator>>()). Тогда пользователь сможет выбрать только из того, что заведомо работает и поддерживается.

А ещё писать целиком - это опасное дело. Приятнее для человека указать одну цифру, чем вручную писать целом слово

GlazProject · 2025-01-03T20:12:31Z

TagsCloudContainer/Tag.cs

+        Font = font;
+        Color = color;
+        var rect = g.MeasureString(Value, Font).ToSize();
+        Frame = new Size(rect.Width + 2, rect.Height + 2);


Кстати, размеры можно считать "снаружи" и передавать уже готовый size. Тогда это станет record

GlazProject · 2025-01-03T20:13:16Z

TagsCloudContainer/Tag.cs

+
+    public Tag(Graphics g, Word word, Font font, Color color)
+    {
+        Value = word.Value;


Точно нужно доставать значение из Word? Можно его сохранить как самодостаточную единицу, вдруг что-то понадобится в него добавить

GlazProject · 2025-01-08T07:44:57Z

Client/Program.cs

+            using var scope = container.BeginLifetimeScope();
+            scope.Resolve<PictureMaker>().DrawPicture();


Можно использовать без усложнений со скоупами, так как жизненный цикл всех сущностей совпадает с временем жизни всего приложения. Просто у контейнера вызывать Resolve

GlazProject · 2025-01-08T07:47:32Z

Client/Program.cs

+using System.Drawing;
+using System.Reflection;
+using System.Threading.Channels;
+using TagsCloudContainer.Configuration;
+using TagsCloudContainer.PointGenerators;
+using TagsCloudContainer.StringParsers;
+using TagsCloudContainer.TextProviders;
+using TagsCloudContainer.WordFilters;
+using System.Linq;
+using System.Security.Cryptography.X509Certificates;
+using System.ComponentModel;
+using TagsCloudContainer;
+using TagsCloudContainer.TextProcessor;
+using Autofac;


Очень много лишних using (

GlazProject · 2025-01-08T07:55:06Z

Client/Program.cs

+            var xLine = Console.ReadLine();
+            var yLine = Console.ReadLine();


Без кода никогда не догадаться, в каком формате вводить координаты. Пользователю нужно давать как можно больше подсказок. Желательно все ReadLine заменить на что-нибудь такое

var xLine = ReadValue("Координата Х"); var yLine = ReadValue("Координата Y");

private static string? ReadValue(string? argName = null) { Console.Write($"{argName ?? ""}: "); return Console.ReadLine(); }

GlazProject · 2025-01-08T08:03:39Z

Client/Program.cs

+            if (colors.ContainsKey(inp))
+            {
+                config.Color = Color.FromName(colors[inp].ToString());


ContainsKey под капотом уже получает значение по ключу, а потом снова происходит поиск по этому же ключу. Здесь это чревато только повторением одного и того же действия. В многопоточном коде это может привести даже к ошибке

поток 1: colors.ContainsKey(inp) => true
поток 2: colors.Remove(inp)
поток 1: colors[inp] => KeyNotFoundException

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

GlazProject · 2025-01-08T08:04:39Z

Client/Program.cs

+                Console.WriteLine("\t" + point.Key);
+            Console.WriteLine("Введите, соблюдая орфографию");
+            var pointGenerator = Console.ReadLine().ToLower();
+            if (pointGenerators.ContainsKey(pointGenerator))


Аналогично цвету, лучше использовать TryGetValue, но не критично здесь

GlazProject · 2025-01-08T09:05:13Z

Client/Program.cs

+            var inp = Console.ReadLine().ToLower();
+            if (colors.ContainsKey(inp))
+            {
+                config.Color = Color.FromName(colors[inp].ToString());


Color.FromName - почти наверняка сработает, так как поддерживает много цветов. Но безопаснее было бы сделать в библиотеке в качестве параметра конструктора у класса GolorProvider значение enum, а потом его уже в switch преобразовывать в цвет. Таким образом, если вместо color придётся использовать какой-нибудь UnixColor, для внешних пользователей ничего не поменяется в использовании

GlazProject · 2025-01-08T09:08:08Z

TagsCloudContainer.Tests/CloudLayoutShould.cs

+    [TestFixture]
+    public class CloudLayoutShould
+    {
+        private CloudLayout layout;


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

GlazProject · 2025-01-08T09:10:08Z

TagsCloudContainer.Tests/RandomColorTagGeneratorShould.cs

+        public void SetRightFontSize()
+        {
+            var bitmap = new Bitmap(1, 1);
+            using var graphics = Graphics.FromImage(bitmap);


Графика нигде не используется

GlazProject · 2025-01-08T09:11:33Z

TagsCloudContainer.Tests/RandomColorTagGeneratorShould.cs

+
+namespace TagsCloudContainer.Tests
+{
+    public class RandomColorTagGeneratorShould


Это ведь тест на два независимых объекта: TagGenerator и RandomColorProvider. Сложно будет понять, из-за кого пошло не так, при последующей разработке

GlazProject · 2025-01-08T09:13:15Z

TagsCloudContainer.Tests/TextProcessorShould.cs

+        public void Process()
+        {
+            var result = new TextProcessor.TextProcessor(
+                new TxtTextProvider(@"TextFile1.txt"), new RegexParser(), new BoringWordFilter(), new ShortWordFilter()).WordFrequencies();


Надежнее сделать какой-нибудь тестовый TextProvider, который будет просто текст из строки возвращать. Это избавит от возможных ошибок из-за работы TxtTextProvider и слова будут на виду в тесте

GlazProject · 2025-01-13T04:02:57Z

TagsCloudContainer/ColorProviders/ColorProvider.cs

+
+public class ColorProvider : IColorProvider
+{
+    [CompilerGenerated] private readonly Color _color;


Думаю, здесь не нужен системный атрибут [CompilerGenerated], ведь этот код пишешь ты сам

d2em0n added 7 commits December 25, 2024 17:25

start

59d38a8

add simple filter

45b91db

add textprocessor

441cd69

add some tests for processor, filter, provider

143db64

add layout with some tests

baf26aa

add interface for TextProcessor

8149754

add Tag and interface for Tag generator

1b85764

GlazProject reviewed Dec 26, 2024

View reviewed changes

d2em0n added 7 commits December 27, 2024 14:16

remove and sort usings, text processor refactoring

3f413ea

Word became record

29ea260

add interface for string parsers

e9254ec

add rnd color tag generator

5259a19

add shortwordfilter

78bb80a

hypothesis

cf302b3

add chosing color

66b48b9

GlazProject reviewed Jan 3, 2025

View reviewed changes

d2em0n added 6 commits January 4, 2025 17:20

add IColorProvider and refactor TagGenerator

c329079

pattern filter for filters

529f90c

container refuse to wark (

821147e

mvp

2392c92

add DocTextProvider

db6772b

DocXTextProvider

29142c4

GlazProject reviewed Jan 8, 2025

View reviewed changes

d2em0n added 7 commits January 11, 2025 16:35

minus unused usings

2cb1edd

fix color and pointgenerator checking

8eeaab8

fix color data integrity

ab0392f

fix random for multithreading in randomcolorprovider

825fa46

no public mod to interface methods

2fa6816

fix TxtText provider

946768e

dispose idisposable

7c1ca09

d2em0n added 2 commits January 12, 2025 17:31

fix regexparser

ef193e7

fix packages

cb067a4

GlazProject reviewed Jan 13, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Бессараб Дмитрий #31

Бессараб Дмитрий #31

d2em0n commented Dec 25, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Jan 3, 2025

GlazProject Dec 26, 2024

GlazProject Dec 26, 2024

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025 •

edited

Loading

GlazProject Jan 3, 2025

GlazProject Jan 3, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 8, 2025

GlazProject Jan 13, 2025

		using var scope = container.BeginLifetimeScope();
		scope.Resolve<PictureMaker>().DrawPicture();

		var xLine = Console.ReadLine();
		var yLine = Console.ReadLine();

Бессараб Дмитрий #31

Are you sure you want to change the base?

Бессараб Дмитрий #31

Conversation

d2em0n commented Dec 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

GlazProject Jan 3, 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

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

GlazProject Jan 3, 2025 •

edited

Loading