Акулин Дмитрий #251
Акулин Дмитрий #251
Conversation
Pasha0666
left a comment
Pasha0666
left a comment
There was a problem hiding this comment.
Перва часть задачи сделана хорошо
Считаю new AssertionScope() что-то около чита. Ну и сильно много всего не стоит в него запихивать, т.к. читаемость от этого в итоге не растет
Часть про Царя тут тоже норм решение, но можно улучшить
И давай еще вести хорошие коммиты. Ну т.е. разбивать их по логическим кускам и давать внятное именование
cs/HomeExercises/ObjectComparison.cs
Outdated
| actualTsar.Should().BeEquivalentTo(expectedTsar, options => options | ||
| .IncludingFields() | ||
| .Excluding(field => field.Path.EndsWith(nameof(Person.Id))) |
There was a problem hiding this comment.
А что будет если появиться еще одно поле с окончанием Id?
There was a problem hiding this comment.
Еще мое любимое про рекурсии тут есть
There was a problem hiding this comment.
А что будет если появиться еще одно поле с окончанием Id?
Тогда мы упустим это новое поле из сравнения. Ну то есть не очень надежное правило получилось конечно.
Переписать?
Про рекурсию это ты видимо имеешь ввиду случай когда у нас зациклены ссылки. Типо есть два царя и каждый является родителем другого. Хоть в теории такого не должно быть, но, как я понял, тесты с цикличными ссылками будут падать по дефолту и нужно добавить .IgnoringCyclicReferences() чтобы такие тесты проходили.
There was a problem hiding this comment.
Ага, переписать историю с Id
Про рекурсию прав, можешь не добавлять
| // 3. Зачастую, намного проще иметь возможность сказать, что сравниваем все поля кроме некоторых, | ||
| // чем перечислять все поля, которые нужно сравнить. |
There was a problem hiding this comment.
Я бы сказал зависит от, но в целом согласен)
2 participants
No description provided.