Conversation
& divided and add test in NumberValidatorTests
MrTimeChip
left a comment
MrTimeChip
left a comment
There was a problem hiding this comment.
Достаточно странные проблемы с форматированием, табуляцией. Можно такое поправить даже авторефакторингом. В остальном - очень даже неплохо, как поправишь, посмотрю снова.
| [TestCase(3, 2, true, "a.sd", false)] | ||
| [TestCase(3, 2, true, "", false)] | ||
| [TestCase(3, 2, true, null, false)] | ||
| public void ValidateNumberCorrect_When_DaraCorrect(int precision, int scale, bool onlyPositive, string value, bool expectedResult) |
| Assert.DoesNotThrow(() => new NumberValidator(1, 0, true)); | ||
| Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, false)); | ||
| Assert.DoesNotThrow(() => new NumberValidator(1, 0, true)); | ||
| [Test] |
There was a problem hiding this comment.
Поехало форматирование - нет табуляции после "{" у класса.
| Assert.DoesNotThrow(() => new NumberValidator(1, 0, true)); | ||
| Assert.Throws<ArgumentException>(() => new NumberValidator(-1, 2, false)); | ||
| Assert.DoesNotThrow(() => new NumberValidator(1, 0, true)); | ||
| [Test] |
There was a problem hiding this comment.
При использовании [TestCase] атрибут [Test] не нужен, вроде как.
| [TestCase(1, -2, false)] | ||
| [TestCase(1, 2, true)] | ||
| [TestCase(1, 1, false)] | ||
| public void Should_ThrowArgumentException_When_InitIncorrectData(int precision, int scale, bool onlyPositive) |
There was a problem hiding this comment.
Не совсем понятно "InitIncorrectData", можно без init.
| public void CheckCurrentTsar() | ||
| { | ||
| var actualTsar = TsarRegistry.GetCurrentTsar(); | ||
| [Test] |
There was a problem hiding this comment.
Тут тоже слетело форматирование - не хватает табуляции.
| [TestCase(17, 2, true, "0.0", true)] | ||
| [TestCase(17, 2, true, "0", true)] | ||
| [TestCase(3, 2, true, "00.00", false)] | ||
| [TestCase(3, 2, true, "-0.0", false)] | ||
| [TestCase(3, 2, true, "+0.00", false)] | ||
| [TestCase(4, 2, true, "+1.23", true)] | ||
| [TestCase(3, 2, true, "+1.23", false)] | ||
| [TestCase(17, 2, true, "0.000", false)] | ||
| [TestCase(3, 2, true, "-1.23", false)] | ||
| [TestCase(3, 2, true, "a.sd", false)] | ||
| [TestCase(3, 2, true, "", false)] | ||
| [TestCase(3, 2, true, null, false)] |
There was a problem hiding this comment.
Все кейсы тут не учитывают один из параметров, он всегда одинакового значения. Давай напишем ещё кейсов, где этот параметр тоже будет изменен?
| [TestCase(17, 2, true, "0.0", true)] | ||
| [TestCase(17, 2, true, "0", true)] | ||
| [TestCase(3, 2, true, "00.00", false)] | ||
| [TestCase(3, 2, true, "-0.0", false)] | ||
| [TestCase(3, 2, true, "+0.00", false)] | ||
| [TestCase(4, 2, true, "+1.23", true)] | ||
| [TestCase(3, 2, true, "+1.23", false)] | ||
| [TestCase(17, 2, true, "0.000", false)] | ||
| [TestCase(3, 2, true, "-1.23", false)] | ||
| [TestCase(3, 2, true, "a.sd", false)] | ||
| [TestCase(3, 2, true, "", false)] | ||
| [TestCase(3, 2, true, null, false)] |
There was a problem hiding this comment.
Давай ещё всем кейсам добавим понятные названия, чтобы можно было понять, что именно конкретный кейс означает с точки зрения логики?
cs/HomeExercises/ObjectComparison.cs
Outdated
| Assert.AreEqual(actualTsar.Height, expectedTsar.Height); | ||
| Assert.AreEqual(actualTsar.Weight, expectedTsar.Weight); | ||
| actualTsar.Should().BeEquivalentTo(expectedTsar, options => | ||
| options.Excluding(option => option.Id) |
There was a problem hiding this comment.
Тут option не совсем корректно, как мне кажется. Здесь мы поля выбираем у объекта, все таки, а не у options.
|
Все хорошо, 2 балла (это максимум). |
2 participants
@DonielPankek