-
Notifications
You must be signed in to change notification settings - Fork 245
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Савенко Дмитрий #226
base: master
Are you sure you want to change the base?
Савенко Дмитрий #226
Conversation
config.For<double>() | ||
.SetCulture(germanCulture)); | ||
|
||
result.Should().Contain("Height = 180,5"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Для этого теста как будто кейсов не хватает, чтобы точно проверить, что он работает.
Предлагаю сделать два TestCase, где будут разные ожидаемые поведения (например, 180.5
и 180,5
).
[Test] | ||
public void PrintToString_WithCollection_ShouldPrintCollectionElements() | ||
{ | ||
testPerson.PrintToString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
В этом тесте действие перемешалось с проверкой - давай всё-таки разделим.
} | ||
|
||
[Test] | ||
public void PrintToString_CustomSerializer_ShouldUseCustomFormat() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Чутка порядка наведём: у тебя есть тест PrintToString_CustomPropertySerializer_ShouldUseCustomFormat
, давай этот тест переименуем, чтобы точно обозначить, что в нём сериализация для типа устанавливается.
} | ||
|
||
[Test] | ||
public void PrintToString_ExcludeDictionary_ShouldNotPrintDictionary() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Этот тест лишний - по сути он дублирует исключение сериализации свойства.
ObjectPrinting/PrintingConfig.cs
Outdated
|
||
parsedObjects[obj] = nestingLevel; | ||
|
||
switch (obj) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ObjectPrinting/PrintingConfig.cs
Outdated
: Serialize(propertyValue, nestingLevel, parsedObjects); | ||
} | ||
|
||
private PropertyInfo GetProperty<TProperty>(Expression<Func<TOwner, TProperty>> memberSelector) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
статический метод
ObjectPrinting/PrintingConfig.cs
Outdated
return Serialize(obj, 0, new Dictionary<object, int>()); | ||
} | ||
|
||
private string Serialize(object obj, int nestingLevel, Dictionary<object, int> parsedObjects) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Тут надо пометить, что obj
может быть null
:
private string Serialize(object? obj, ...)
Отсюда же вытекает, что коллекции TypeSerializers
и PropertySerializers
могут содержать nullable
типы:
public Dictionary<Type, Func<object?, string>> TypeSerializers { get; } = new();
public Dictionary<PropertyInfo, Func<object?, string>> PropertySerializers { get; } = new();
Если это всё проделать, то останется одно неочевидное место поведения в методе SerializeProperty
:
Convert.ToString
может вернуть null
, а не строку - непонятно, как это в итоге отразиться на всей логике.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Добавил
Convert.ToString(propertyValue, culture) ?? "null"
private readonly PrintingConfig<TOwner> _config; | ||
private readonly PropertyInfo _propertyInfo; | ||
|
||
public PropertyPrintingConfig(PrintingConfig<TOwner> config, PropertyInfo propertyInfo) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Можно использовать PrimaryConstructor:
public class PropertyPrintingConfig<TOwner, TProperty>(PrintingConfig<TOwner> config, PropertyInfo propertyInfo)
{
...
}
config.For(p => p.Scores) | ||
.UseSerializer(dict => | ||
{ | ||
var scores = (Dictionary<string, int>)dict; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Зачем тут этот каст? Вроде и без него работает...
@SlavikGh0st