Conversation
GlazProject
left a comment
GlazProject
left a comment
There was a problem hiding this comment.
Получилось весьма неплохо. Код приятный для чтения, разумно разбит на методы, которое переиспользуются. Хочу обратить внимание на работу с рефлексией: получение типа очень медленное, поэтому его стоит выполнять как можно реже. Это очень сильный инструмент, а с ним приходит и большая ответственность
| return this; | ||
| } | ||
|
|
||
| private MemberInfo GetMemberInfo<TProp>(Expression<Func<TOwner, TProp>> memberSelector) |
There was a problem hiding this comment.
Если метод не обращается к экземпляру класса, то лучше выносить в статичные. Это чуть улучшает оптимизацию
| { | ||
| var body = memberSelector.Body; | ||
| if (body is MemberExpression memberExpr) | ||
| return memberExpr.Member; |
There was a problem hiding this comment.
В целом здесь можно ещё выполнять проверку, что DeclaringType соответствует TOwner, но не обязательно. Проверка поможет отловить вот такое использование:
var node = new Node(10);
var result = ObjectPrinter.For<Person>()
.For(p => node.Id).Using(i => $"Incorrect: {i}")
.PrintToString(person);
| private readonly MemberInfo memberInfo; | ||
| private readonly PrintingConfig<TOwner> parent; |
There was a problem hiding this comment.
Оба этих поля уже есть в родительском классе
|
|
||
| public class MemberPrintingConfig<TOwner, TProp> | ||
| { | ||
| protected readonly PrintingContext context; |
There was a problem hiding this comment.
protected у нас чаще пишутся с заглавной буквы, но это вопрос принятого стиля
| _ => throw new ArgumentException("Member must be a property or field") | ||
| }; | ||
|
|
||
| string ApplyTrim(string s) => s.Length <= maxLength ? s : s[..maxLength]; |
There was a problem hiding this comment.
Лучше избегать inline методов, их очень тяжело читать. Здесь можно смело выделять отдельный приватный метод и использовать его
| @@ -0,0 +1,12 @@ | |||
| namespace ObjectPrinting.Tests.Data; | |||
|
|
||
| [Test] | ||
| [UseReporter(typeof(DiffReporter))] | ||
| public void PrintToString_ShouldApplyCultureForType() |
There was a problem hiding this comment.
Этот тест не очень показателен, потому что в пользователеле всего одно свойство с таким типом. Стоит завести два и проверять, что здесь для всех применилось, а в мембере только к конкретному
|
|
||
| [Test] | ||
| [UseReporter(typeof(DiffReporter))] | ||
| public void PrintToString_ShouldApplyMultipleSerializersToMember() |
There was a problem hiding this comment.
А этот тест проверяет сразу применение нескольких модификаторов строк и работу Trim. Лучше разделять разные сценарии на разные тесты
|
|
||
| [Test] | ||
| [UseReporter(typeof(DiffReporter))] | ||
| public void PrintToString_ShouldSerializeDictionary() |
There was a problem hiding this comment.
Не хватает ещё теста, когда ключ в словаре - это сложный тип
| else | ||
| sb.Append(PrintToString(entry.Key, nestingLevel + 1)); | ||
|
|
||
| sb.Append("] = "); |
There was a problem hiding this comment.
Перед закрытием скобки как-будто не хватает переноса строки в случае сложного типа
Dictionary`2
[Node
Id = 1
Child = null] = Person
Id = 00000000-0000-0000-0000-000000000000
Name = null
Height = 0
Age = 0
Surname = null
2 participants
@GlazProject