-
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
Кашин Александр #22
base: master
Are you sure you want to change the base?
Кашин Александр #22
Conversation
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.
Промежуточное ревью
@@ -0,0 +1,61 @@ | |||
namespace HomeExercise.Tasks.ObjectComparison; | |||
|
|||
public class TestPersonBuilder |
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.
Молодец, что сделал билдер для тестов, код стал читаемее :)
|
||
public class TestPersonBuilder | ||
{ | ||
private const string DEFAULT_NAME = ""; |
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.
Думаю тогда лучше значения по умолчанию присваивать сразу же полям класса или же вовсе убрать их, если нет специфичных значений. В таком случае можно будет сразу же сделать Create().Build()
без вызова методов настройки полей.
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.
Убрал явное присваивание значений по-умолчанию.
Testing/Basic/Homework/1. ObjectComparison/TestPersonBuilder.cs
Outdated
Show resolved
Hide resolved
Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs
Outdated
Show resolved
Hide resolved
Testing/Basic/Homework/2. NumberValidator/NumberValidatorTests.cs
Outdated
Show resolved
Hide resolved
Добавил константы для текстовок, перенес повторяющиеся проверки в TestCase
Исключил Id из проверки в наследниках, убрал лишние константы
No description provided.