Медведев Фома by Kpokoko · Pull Request #240 · kontur-courses/fluent-api

Kpokoko · 2025-11-16T15:40:37Z

@tripples25
Выполнил минимальные требования по задаче

tripples25 · 2025-11-18T19:46:46Z

ObjectPrinting/Tests/ObjectPrinterAcceptanceTests.cs

+            person.Phone = new Phone {Name = "Смартфон Vivo", Owner = person};

-            var printer = ObjectPrinter.For<Person>();
+            var printer = ObjectPrinter.For<Person>()


Сейчас из интерфейса торчит куча методов. Пользоваться таким будет сложно. Давай подумаем, как можно убрать часть из этих вещей, чтобы они не торчали наружу. Тут пока вижу два способа - как-то изменить способ работы Serializaers(возможно вернуть логику внутрь конфига), либо закрыть внутренние методы ключевым словом internal, но тут возникнет проблема расширяемости.

Тут ещё и доступен TrimStringToLength, который по факту должен быть доступен только для типа string

tripples25 · 2025-11-18T20:06:53Z

ObjectPrinting/PrintingConfig.cs

+        public readonly Dictionary<Type, CultureInfo> CustomCultureInfos;
+        public readonly Dictionary<string, ISerializer> PropertySerializers;
+        public readonly Dictionary<string, int> MaxStringLength;
+        public readonly HashSet<string> ExcludedProperties;


Давай для всех полей сделаем модификаторы private, а также заиспользуем ридонли коллекции (IReadOnlyDictionary, IReadOnlySet). Следует инкапсулировать данные, которые нужны для внутренней логики, а доступ к ним осуществлять через методы

tripples25 · 2025-11-18T20:32:32Z

ObjectPrinting/PrintingConfig.cs

+        public PrintingConfig<TOwner> ExcludePropertyOfType<T>()
+        {
+            var tempSet = new HashSet<Type>(ExcludedTypes);
+            tempSet.Add(typeof(T));


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

tripples25 · 2025-11-18T20:33:33Z

ObjectPrinting/PrintingConfig.cs

+        public PrintingConfig<TOwner> WithTypeSerializtionType<T>(ISerializer serializer)
+        {
+            var tempDict = new Dictionary<Type, ISerializer>(TypeSerializers);
+            tempDict.Add(typeof(T), serializer);


Здесь упадём с ошибкой, если данный тип уже есть в словаре. Кажется стоит это как-то обрабатывать. Ниже в методах та же проблема

tripples25 · 2025-11-18T20:39:22Z

ObjectPrinting/PrintingConfig.cs

+                ExcludedProperties);
+        }
+
+        public PrintingConfig<TOwner> TrimStringToLength(string propertyName, int length)


Тут должны давать этот метод вызывать только для строковых значений

tripples25 · 2025-11-18T20:53:02Z

ObjectPrinting/Serializers.cs

+                continue;
+            }
+
+            sb.Append(identation + propertyInfo.Name + " = " +


Вот эта строчка практически три раза повторяется в коде, с небольшими различиями. Может вынесем её в отдельный метод?

tripples25 · 2025-11-18T20:54:53Z

ObjectPrinting/Serializers.cs

+    }
+}
+
+public class GuidSerializer : ISerializer


Давай разнесём сериализаторы по разным файликам. А ещё, кажется это не сильно удобно - создавать отдельный класс, чтобы сериализовать поле. Может попробуем как-нибудь иначе построить контракт?

tripples25 · 2025-11-18T20:55:49Z

ObjectPrinting/PropertyPrintingConfig.cs

+    private readonly PrintingConfig<TOwner> _config;
+    private Expression<Func<TOwner, TPropType>>? _propertySelector;
+
+    public PropertyPrintingConfig(PrintingConfig<TOwner> config, Expression<Func<TOwner, TPropType>>? propertySelector)


Этот Expression никак не валидируется, по факту я могу туда засунуть что угодно

tripples25 · 2025-11-18T20:59:56Z

ObjectPrinting/Tests/ObjectPrinterAcceptanceTests.cs

                //3. Для числовых типов указать культуру
+                .SetTypeCulture<double>(CultureInfo.InvariantCulture)
                //4. Настроить сериализацию конкретного свойства
+                .GetProperty(x => x.Name).SetSerializationType(new NameSerializer())


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

tripples25 · 2025-11-18T21:02:34Z

ObjectPrinting/Tests/ObjectPrinterAcceptanceTests.cs

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


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

tripples25 · 2025-11-23T12:54:34Z

ObjectPrinting/Serializers.cs

+public interface ISerializer
+{
+    public Func<object?, int, int, string> SerializerFunc { get; }
+    public Func<object?, int, int, string> GetSerializerFunc => SerializerFunc;


Не используется

tripples25 · 2025-11-23T12:56:44Z

ObjectPrinting/Serializers.cs

+    public Func<object?, int, int, string> GetSerializerFunc => SerializerFunc;
+}
+
+public class Serializer : ISerializer


Кажется в этом классе теперь не сильно много смысла и вместо его использования можно было просто использовать Func

tripples25 · 2025-11-23T13:02:12Z

ObjectPrinting/Serializers.cs

+    public Func<object?, int, int, string> SerializerFunc { get; }
+    public Func<object?, int, int, string> GetSerializeFunc => SerializerFunc;
+
+    public Serializer(Func<object?, int, int, string> serializerFunc)


Контракт входной функции не понятен - требуется передать функцию от объекта, числа и числа. Без понимания как это используется другими классами не понять для чего нужны эти числа, да и в итоге они могут никак и не применяться. Если ты хотел всё-таки как-то это обыграть, то числа скорее должны были передаваться отдельно под понятными именами. То есть примерно так:

public Serializer(Func<object?, string> serializator, int identation, int deepnessLevel)

Но как мне кажется это не ответственность конкретного сериализатора работать с этими значениями

Я решил просто ликвидировать и интерфейс, и этот класс, и оставить просто Func<>, обёрнутый в делегат. Что-то такое будет нормальным вариантом?

Да, вполне. Либо можно оставить Func<object, string>. То и то будет норм вариантом в целом

tripples25 · 2025-11-23T13:04:07Z

ObjectPrinting/PropertyPrintingConfigExtensions.cs

+public static class PropertyPrintingConfigExtensions
+{
+    public static PrintingConfig<TOwner> SetSerializationStyle<TOwner, TPropType>
+        (this PropertyPrintingConfig<TOwner, TPropType> propertyConfig, Serializer serializer)


Здесь почему-то с классом работаешь, а не с интерфейсом. Я не смогу сюда передать другой наследник ISerializer

tripples25 · 2025-11-23T13:14:25Z

ObjectPrinting/PropertyPrintingConfig.cs

+    }
+
+    public PrintingConfig<TOwner> ParentConfig => config;
+    public Expression<Func<TOwner, TPropType>>? PropertySelector => propertySelector;


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

Я посидел-поделал, но мыслей, как это реализовать не пришло, кроме как попытаться убрать Extensions и сделать эти методы приватными, но тогда возникает другой вопрос: как мне ограничить вызов метода Trim только для строк, кроме как where T : string, ведь так тут нельзя. Как тут поступить стоит: развивать эту мысль, или подумать над другими способами?

Так я не совсем про то. Сейчас у тебя торчат наружу свойства PropertyName, PropertySelector и ParentConfig. Это засоряет список, их стоит сделать приватными

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

Ага, понял. Попробуй подумать про принципы ООП и что из них мы можем использовать.

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

tripples25 · 2025-11-23T14:20:46Z

ObjectPrinting/Tests/ObjectPrinterAcceptanceTests.cs

    {
        [Test]
-        public void Demo()
+        public void DemonstrationTest()


Не хватает тестов на валидации

tripples25 · 2025-11-23T14:29:29Z

ObjectPrinting/PrintingConfig.cs

+            return sb.ToString();
+        }
+
+        private string SerializeDictionaryElement(object obj, int deepnessLevel)


А не логичнее тут работать с KeyValuePair или чем-то более удобным? Сейчас из-за этого приходится тебе хардкодить доступ к объектам в паре

tripples25 · 2025-11-23T14:29:57Z

ObjectPrinting/PrintingConfig.cs

+                return deepnessExceededString;
+            var type = obj.GetType();
+            var key = type.GetProperty("Key")!.GetValue(obj);
+            var value = type.GetProperty("Value")!.GetValue(obj);


На значения словаря не будут применяться правила сериализации?

tripples25 · 2025-11-23T14:30:10Z

ObjectPrinting/PrintingConfig.cs

+            if (IsDeepnessOverflow(deepnessLevel))
+                return deepnessExceededString;
+            var type = obj.GetType();
+            var key = type.GetProperty("Key")!.GetValue(obj);


И на ключи видимо тоже?

tripples25 · 2025-11-23T14:31:36Z

ObjectPrinting/Tests/Person.cs

@@ -1,3 +1,3 @@
 using System;

 namespace ObjectPrinting.Tests


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

👍 (на приватные поля и свойства добавил отдельный тест, сделал Age публичным полем, теперь сериализуется)

Kpokoko added 2 commits November 6, 2025 20:14

Исключение свойств типа и кастомная сериализация

a6cdbdf

Завершил реализацию базовых методов и добавил тесты

85c2ea9

tripples25 suggested changes Nov 18, 2025

View reviewed changes

Kpokoko added 3 commits November 19, 2025 12:09

Мини-коммит перед возвратом логики из Serializer.cs

b9b5983

Отрефакторил по замечаниям

a2bf07c

Добавил сериализацию базовых коллекций

3cd8377

tripples25 suggested changes Nov 23, 2025

View reviewed changes

Kpokoko added 2 commits November 24, 2025 22:22

Исправил замечания (кроме одинаковых имён и словарей)

948d9e1

Закрыл оставшиеся замечания (словари, сериализация свойств)

483f276

Conversation

Kpokoko commented Nov 16, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Kpokoko Nov 24, 2025 •

edited

Loading