Skip to content
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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

MrSasha00
Copy link

No description provided.

Copy link

@PeterMotorniy PeterMotorniy left a 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

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 = "";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Кажется, эти константы не нужны, можно вынести дефолтные значения в сами методы

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Думаю тогда лучше значения по умолчанию присваивать сразу же полям класса или же вовсе убрать их, если нет специфичных значений. В таком случае можно будет сразу же сделать Create().Build() без вызова методов настройки полей.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Да, давай поля тогда присваивать

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Убрал явное присваивание значений по-умолчанию.

kashin.aleksandr added 3 commits November 5, 2024 22:18
Добавил константы для текстовок, перенес повторяющиеся проверки в TestCase
Исключил Id из проверки в наследниках, убрал лишние константы
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants