Skip to content

Семёнов Дмитрий#57

Open
dm7672 wants to merge 4 commits intokontur-courses:masterfrom
dm7672:master
Open

Семёнов Дмитрий#57
dm7672 wants to merge 4 commits intokontur-courses:masterfrom
dm7672:master

Conversation

@dm7672
Copy link

@dm7672 dm7672 commented Oct 29, 2025

No description provided.

Comment on lines 19 to 22
actualTsar.Should().BeEquivalentTo(expectedTsar, options =>
options.Excluding(person => person.Id)
.Excluding(person => person.Parent.Id)
);

Choose a reason for hiding this comment

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

А person.Parent.Parent.Id? :) и тд

Comment on lines 23 to 28
// Недостатки подхода CustomEquality по сравнению с решением выше в том что
// 1. Используется проверка на истинность AreEqual, в случае ошибки покажет только то что AreEqual вернуло False
// 2. Кастомный Ассерт ниже нерасширяем, при попытке добавить новое поле в класс придётся переписывать его
// 3. Читаемость гораздо хуже, в ассерте могла быть пропущена проверка поля и внешне он выглядел бы также,
// а значит чтобы понять что пытаемся проверить пришлось бы читать весь код
// 4. Код выше короче

Choose a reason for hiding this comment

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

Давай еще подумаем что может быть не так именно с реализацией AreEquals.

Comment on lines 23 to 28
// Недостатки подхода CustomEquality по сравнению с решением выше в том что
// 1. Используется проверка на истинность AreEqual, в случае ошибки покажет только то что AreEqual вернуло False
// 2. Кастомный Ассерт ниже нерасширяем, при попытке добавить новое поле в класс придётся переписывать его
// 3. Читаемость гораздо хуже, в ассерте могла быть пропущена проверка поля и внешне он выглядел бы также,
// а значит чтобы понять что пытаемся проверить пришлось бы читать весь код
// 4. Код выше короче

Choose a reason for hiding this comment

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

Сомнительное преимущество :) Попробуй перефразировать

@@ -8,24 +9,143 @@ namespace HomeExercise.Tasks.NumberValidator;
public class NumberValidatorTests

Choose a reason for hiding this comment

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

Очень много похожего кода. Подумай как соблюсти DRY

Choose a reason for hiding this comment

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

Из названий методов непонятно что именно мы тестируем.
Давай использовать паттерн названий Method_ShouldReturn_When

Например, IsValidNumber_ShouldReturnTrue_WhenZero

Choose a reason for hiding this comment

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

Давай еще тестов накинем, пока маловато получается. Например ".", ",1", "\n".

Это же строчка, значений можно много придумать.

act.Should().Throw<ArgumentException>();
}
[Test]
public void ThrowsArgumentException_WhenScaleMoreOrEqualToPrecision()

Choose a reason for hiding this comment

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

Так тесты лучше не смешивать. У тебя есть отельный случай, когда scale == precision, и отдельный случай, когда scale > precision.

Представь, что у тебя сломалась логика валидации для scale > precision. Твой тест это не отловит.

Comment on lines 12 to 22
public void ThrowsArgumentException_WhenPrecisionNegative()
{
Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, true));
Assert.DoesNotThrow(() => new NumberValidator(1, 0, true));
Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, false));
Assert.DoesNotThrow(() => new NumberValidator(1, 0, true));
Action act = () => new NumberValidator(-1, 2);
act.Should().Throw<ArgumentException>();
}
[Test]
public void ThrowsArgumentException_WhenScaleMoreOrEqualToPrecision()
{
Action act = () => new NumberValidator(2, 2);
act.Should().Throw<ArgumentException>();
}

Choose a reason for hiding this comment

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

Тесты на валидацию в конструкторе не покрывают условия

Comment on lines 24 to 28
public void Creates_NumberValidator()
{
Action act = () => new NumberValidator(1, 0, true);
act.Should().NotThrow();
}

Choose a reason for hiding this comment

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

А надо ли это тестировать? У тебя буквально все тесты создают NumberValidator с какими-то значениями и не падают.

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