-
Notifications
You must be signed in to change notification settings - Fork 281
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
Азанов Андрей #243
base: master
Are you sure you want to change the base?
Азанов Андрей #243
Conversation
cs/HomeExercises/ObjectComparison.cs
Outdated
//Assert.AreEqual(expectedTsar.Parent.Height, actualTsar.Parent.Height); | ||
//Assert.AreEqual(expectedTsar.Parent.Parent, actualTsar.Parent.Parent); | ||
|
||
actualTsar.Should().Be(expectedTsar, new TsarEqualityComparer()); |
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.
Круто, что разобрался с компаратором, но почему не Should().BeEquivalentTo(...)
?
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.
IEqualityComparer можно переиспользовать в будущем, притом не только во FluentAssertions.
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.
Но при этом остается проблема с поддержкой новых появляющихся полей, каждое из них необходимо будет не забыть внести в компаратор. Еще момент - когда используешь подобные конструкции, их тоже по хорошему нужно покрывать минимальными тестами. В общем, рекомендую откатиться к BeEquivalentTo
, который создан ровно для этой проблемы, позволяет настроить вложенность и защититься от бесконечной рекурсии
cs/HomeExercises/ObjectComparison.cs
Outdated
&& x.Age == y.Age | ||
&& x.Height == y.Height | ||
&& x.Weight == y.Weight | ||
&& Equals(x.Parent, y.Parent); |
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.
В обоих решениях, в старом и твоем, есть рекурсивный вызов. Может ли это вызвать какие-нибудь проблемы?
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.
Проблемы есть. У рекурсии нет ни базы, ни ограничения на глубину. В задаче человек и родитель как бы образуют односвязный список, и хоть с логической точки зрения он не должен быть замкнутым, случается всякое. Можно получить StackOverflowException. Ещё проверка всего списка неэффективное решение.
} | ||
|
||
[Test] | ||
public void ShouldBeValidNumber_WhenOnePlusSign() |
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
? Получится сильно уменьшить количество кода и сгруппировать похожие тесты
@a_zhelonkin