-
Notifications
You must be signed in to change notification settings - Fork 282
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
Sibogatov Rinat #244
base: master
Are you sure you want to change the base?
Sibogatov Rinat #244
Conversation
cs/HomeExercises/ObjectComparison.cs
Outdated
@@ -14,17 +14,10 @@ public void CheckCurrentTsar() | |||
|
|||
var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70, | |||
new Person("Vasili III of Russia", 28, 170, 60, null)); | |||
|
|||
actualTsar.Should().BeEquivalentTo(expectedTsar, options => options | |||
.Excluding(member => member.SelectedMemberInfo.Name.ToLower().Contains("id"))); |
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.
Не очень хороший вариант, а если добавятся еще какие-то поля, в названии которых будет Id, но их не нужно будет исключать? Поэтому тут я бы предложил просто использовать ==
public void isValidNumber_ShouldBeTrue_WhenNumberWithSigns(string number, bool result) | ||
{ | ||
var validator = new NumberValidator(7, 3, true); | ||
validator.IsValidNumber(number).Should().Be(result); | ||
} | ||
} | ||
|
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.
Вынести в отдельный файл класс ниже, почему-то не могу к нему коммент оставить. На шпоре, имхо, важно не только смотреть на то место, куда ты вносишь изменения но и на весь проект в целом, если видишь что где-то сделано не очень хорошо - исправляй. Так работает и в промышленной разработке (за исключением случаев, когда код написан настолько плохо, что трогать его страшно)
@raccoon6996