-
Notifications
You must be signed in to change notification settings - Fork 32
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
Галичев Артем #13
base: master
Are you sure you want to change the base?
Галичев Артем #13
Changes from 2 commits
c141b1f
3ff842c
9dcb67f
d948c71
80437c9
5a5622f
589aa08
ec56aff
ee134c2
f0f22f6
9a1a895
8aaed3b
5f49f1d
c655d53
4ce8a14
5e0a291
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
using NUnit.Framework; | ||
using FluentAssertions; | ||
using NUnit.Framework; | ||
using NUnit.Framework.Legacy; | ||
|
||
namespace HomeExercise.Tasks.ObjectComparison; | ||
|
@@ -13,17 +14,8 @@ public void CheckCurrentTsar() | |
|
||
var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70, | ||
new Person("Vasili III of Russia", 28, 170, 60, null)); | ||
|
||
// Перепишите код на использование Fluent Assertions. | ||
ClassicAssert.AreEqual(actualTsar.Name, expectedTsar.Name); | ||
ClassicAssert.AreEqual(actualTsar.Age, expectedTsar.Age); | ||
ClassicAssert.AreEqual(actualTsar.Height, expectedTsar.Height); | ||
ClassicAssert.AreEqual(actualTsar.Weight, expectedTsar.Weight); | ||
|
||
ClassicAssert.AreEqual(expectedTsar.Parent!.Name, actualTsar.Parent!.Name); | ||
ClassicAssert.AreEqual(expectedTsar.Parent.Age, actualTsar.Parent.Age); | ||
ClassicAssert.AreEqual(expectedTsar.Parent.Height, actualTsar.Parent.Height); | ||
ClassicAssert.AreEqual(expectedTsar.Parent.Parent, actualTsar.Parent.Parent); | ||
|
||
actualTsar.IsEqual(expectedTsar); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. Понял, корректно) |
||
} | ||
|
||
[Test] | ||
|
@@ -35,6 +27,14 @@ public void CheckCurrentTsar_WithCustomEquality() | |
new Person("Vasili III of Russia", 28, 170, 60, null)); | ||
|
||
// Какие недостатки у такого подхода? | ||
/* | ||
* 1) Если у класса изменять поля (удалять или добавлять), то также придется изменять | ||
This comment was marked as resolved.
Sorry, something went wrong. |
||
* логику проверки на соответствие сущностей. В случае с удалением поля программа вообще не скомпилируется | ||
* 2) Возможна бесконечная рекурсия | ||
* 3) При падении теста не поймем в чем была причина. Ожидался True, а получили False, вот и вся инфа( | ||
* 4) Использование FluentAssertion ускоряет понимание логики секции проверки, за счет использования | ||
* method chaining. В данном тесте вникнуть в логику проверки теста сложнее, чем в первом. | ||
*/ | ||
ClassicAssert.True(AreEqual(actualTsar, expectedTsar)); | ||
} | ||
|
||
|
@@ -50,3 +50,16 @@ private bool AreEqual(Person? actual, Person? expected) | |
&& AreEqual(actual.Parent, expected.Parent); | ||
} | ||
} | ||
|
||
internal static class ObjectComparisonAssertExtensions | ||
{ | ||
public static void IsEqual(this Person actualTsar, Person expectedTsar) | ||
{ | ||
actualTsar.Should().BeEquivalentTo( | ||
expectedTsar, | ||
options => options | ||
.Excluding(x => x.Name == nameof(Person.Id) && x.DeclaringType == typeof(Person)) | ||
.AllowingInfiniteRecursion() | ||
.IgnoringCyclicReferences()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
namespace HomeExercise.Tasks.NumberValidator; | ||
|
||
public class TestNumberValidatorBuilder | ||
This comment was marked as resolved.
Sorry, something went wrong. |
||
{ | ||
public const int DEFAULT_PRECISION = 5; | ||
public const int DEFAULT_SCALE = 3; | ||
public const bool DEFAULT_ONLY_POSITIVE = true; | ||
private int precision = DEFAULT_PRECISION; | ||
private int scale = DEFAULT_SCALE; | ||
private bool onlyPositive = DEFAULT_ONLY_POSITIVE; | ||
|
||
public TestNumberValidatorBuilder WithPrecision(int precision) | ||
{ | ||
this.precision = precision; | ||
return this; | ||
} | ||
|
||
public TestNumberValidatorBuilder WithScale(int scale) | ||
{ | ||
this.scale = scale; | ||
return this; | ||
} | ||
|
||
public TestNumberValidatorBuilder WithOnlyPositive(bool onlyPositive) | ||
{ | ||
this.onlyPositive = onlyPositive; | ||
return this; | ||
} | ||
|
||
public NumberValidator Build() | ||
{ | ||
return new NumberValidator(precision, scale, onlyPositive); | ||
This comment was marked as resolved.
Sorry, something went wrong.
This comment was marked as resolved.
Sorry, something went wrong. |
||
} | ||
} |
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.
Привет) давай сначала общие рекомендации:
Именование коммитов) Имена должны быть емкими (и желательно на английском языке, как стандарте в ИТ индустрии) у меня в github desktop'e они даже не влезают в строчку имени и уезжают в описание
Содержание коммитов должно быть атомарным. Один коммит - одно логическое изменение. Например, в коммите
NumberValidatorTests. Написал недостающие тесты, разделил разные тестовые случаи на разные тестовые методы. Создал класс TestNumberValidatorBuilder для удобного создания класса NumberValidator
ты уже сам видишь, что завел много различных независимых изменений, которые просто завернул в один коммит и отправил. Рекомендую стараться разделять независимое. Зачем это нужно? Чтобы по истории коммитов, понимать, что произошло. Или уметь ревертить только "плохие" части. Или уметь их черри-пикать. И тдПрямо сейчас что-либо делать с коммитами не надо. Это рекомендации на будущее
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.
Также могу порекомендовать отмечать мои сообщения какой-нибудь эмодзи, чтобы отмечать проработанное и не запутаться перед отправкой на следующую итерацию ревью)