Conversation
| var actual = setUpPerson.PrintToString(); | ||
|
|
||
| actual.Should().NotBeNullOrEmpty(); | ||
| actual.Should().Contain("Id = f14dd761-3260-4463-a4ad-6ba14de2026c"); |
There was a problem hiding this comment.
А давай в эти строчки, где есть значения, положим значения из setUpPerson, чтобы руками не править в двух местах потом
| actual.Should().Contain("Id = f14dd761-3260-4463-a4ad-6ba14de2026c"); | ||
| actual.Should().Contain("Person"); | ||
| actual.Should().Contain("Name = Fai"); | ||
| actual.Should().Contain("Age = 20"); |
There was a problem hiding this comment.
А перетащи Height выше Age, чтобы порядок полностью совпал с порядком в Person. Ни на что не влияет, но так попроще читать)
| actual.Should().Contain("Name = Fai"); | ||
| actual.Should().Contain("Age = 20"); | ||
| actual.Should().Contain("Height = 160,5"); | ||
| actual.Should().Contain("Tags = List"); |
|
|
||
|
|
||
| [Test] | ||
| public void PrintToString_ShouldApplyMemberExclusion_WhenNameExcludedInConfig() |
There was a problem hiding this comment.
Название теста не совсем правильное, WhenNameExcluded не совсем явно о том, что мы на самом деле тестируем исключение конкретного поля, вдруг мы только Name умеем исключать
| } | ||
|
|
||
| [Test] | ||
| public void PrintToString_ShouldApplyTypeExclusion_WhenIntTypeExcludedInConfig() |
There was a problem hiding this comment.
Тут тоже я бы предложил убрать Int из названия, мы же тестируем исключение типов. А еще было бы классно завести вторую интовую переменную и её тоже проверить, что мы исключили обе, а не первую попавшуюся
| sb.Append(prop.Name); | ||
| sb.Append(" = "); |
There was a problem hiding this comment.
А почему это в одну строчку не обьединить? И ниже тоже
| if (!t.IsGenericType) return t.Name; | ||
|
|
||
| var defName = t.Name; | ||
| var backtick = defName.IndexOf('`'); | ||
| if (backtick > 0) defName = defName.Substring(0, backtick); |
There was a problem hiding this comment.
Тут тоже стоит вынести тело условия на отдельную строчку и для return-а и для просто переменной. И можно заюзать и range indexer [..backtick]
| return s.EndsWith(Environment.NewLine) ? s : s + Environment.NewLine; | ||
| } | ||
|
|
||
| private static string InvokeSerializer(Delegate del, object value) |
There was a problem hiding this comment.
Вызовы функций в рефлексии, да и сама рефлексия очень хитрая тема, с которой вообще практически не работаешь в обычной жизни, поэтому всегда забывается. Поэтому всегда ставь её по 10 раз под сомнение, а сейчас есть еще и достаточно умные ллмки которые помогут быстро проверить, что же ты написал и окей ли это ;)
Нюансов там много, вспоминается, что там есть какой-то прикол с компиляцией функций рефлексии или в рантайме или в момент сборки, что тоже сильно влияет на производительность, но тут кажется не совсем в этом проблема
Тут например такая штука (это если что ответ ллмки, не мой)
DynamicInvoke медленный, потому что это универсальный механизм рефлексии: при каждом вызове он создает в памяти массив для аргументов (нагружая GC), проверяет соответствие типов в runtime и использует сложную логику для передачи данных из массива в регистры процессора. Прямой вызов делегата (через Func или Action) быстрый, потому что компилятор заранее знает сигнатуру метода; это компилируется в одну процессорную инструкцию (callvirt) без создания лишних объектов, без проверок типов и с возможностью JIT-компилятора оптимизировать или даже встроить (inline) этот код.
Простым языком, если захочется оптимизировать, то нужно будет сменить типы у сериализаторов на вот это, но явно требовать сделать не буду, кажется в целом и так читабельно, но помнить об этом стоит)
internal readonly Dictionary<Type, Func<object, string>> CustomTypeSerializers = new();
internal readonly Dictionary<MemberInfo, Func<object, string>> CustomMemberSerializers = new();
И переписать юзинги с вот такими врапперами
Func<object, string> wrapper = obj => func((TMemberType)obj);
| return value + Environment.NewLine; | ||
| } | ||
|
|
||
| private string ApplyTrimming(string s, MemberInfo? parentMember) |
There was a problem hiding this comment.
Лучше для строчек тоже использовать нулабельность через ?, так можно немного упростить себе жизнь с проверками на null, где они нужны а где нет
| return sb.ToString(); | ||
| } | ||
|
|
||
| private void Serialize(object? obj, StringBuilder sb, int level, MemberInfo? parentMember) |
There was a problem hiding this comment.
А попробуй декомпозировать метод на подметоды внутри. 130 строчек уже кажется многовато и стоит подумать над разбиением на блоки
2 participants
@Yrwlcm