Skip to content
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

Калентьев Илья tg: @m1nus0ne #237

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

m1nus0ne
Copy link

No description provided.

Comment on lines +3 to +9
Name = Mary
Surname = null
Age = 0
Height = 0
BirthDate = 01.01.0001 0:00:00
Address = null
Parent = null
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Прикольно, что ты отдельно обрабатываешь сериализацию словаря, но тогда тут должны быть отступы

Comment on lines +137 to +155
[Test]
public void ObjectPrinter_ShouldCorrectPrintArray()
{
var list = new[] { new Person { Name = "Mary" }, new Person { Name = "Alice" } };

var printer = ObjectPrinter.For<Person[]>();

Approvals.Verify(printer.PrintToString(list));
}

[Test]
public void ObjectPrinter_ShouldCorrectPrintList()
{
var list = new List<Person> { new Person { Name = "Mary" }, new Person { Name = "Alice" } };

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

Approvals.Verify(printer.PrintToString(list));
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Для тестов сериализации коллекций не хватает approved файлов. Сериализация коллекций выглядит странно, например, если в классе будет поле с массивом чисел, они выведутся подряд без разделителей


namespace ObjectPrinting;

public static class ObjectPrintingExtension
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нет изменений ObjectPrinter`а, поэтому напишу здесь, класс можно было сделать static. Создавать экземпляры класса сейчас не имеет смысла

Comment on lines +26 to +27
typeof(int), typeof(double), typeof(float), typeof(string),
typeof(DateTime), typeof(TimeSpan)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Не хватает сериализации long и bool, в c# много различных примитивов, но эти часто используются, поэтому их стоило добавить

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants