Skip to content

Зубков Андрей #231

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.

Sign up for GitHub

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

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

Зубков Андрей #231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
aade570
Refactor ObjectComparison.cs
HardreaM Nov 25, 2023
4efe032
Refactor NumberValidatorTests.cs
HardreaM Nov 26, 2023
4d53576
Auto reformat code
HardreaM Nov 26, 2023
3ca31be
Change property excluding in ObjectComparison.cs
HardreaM Nov 28, 2023
b0f6902
Refactor and add more test cases to NumberValidatorTests.cs
HardreaM Nov 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
120 changes: 92 additions & 28 deletions cs/HomeExercises/NumberValidatorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,100 @@

namespace HomeExercises
{
[TestFixture]
public class NumberValidatorTests
Comment thread
dmnovikova marked this conversation as resolved.
{
[TestCase(-1, 2, TestName = "precision is non-positive")]
[TestCase(null, 2, TestName = "precision is null")]
[TestCase(1, -1, TestName = "scale is negative")]
[TestCase(1, 2, TestName = "scale > precision")]
[TestCase(1, 1, TestName = "scale = precision")]
public void NumberValidator_CreateWithIncorrectParams_ThrowsException(int precision, int scale)
{
Action action = () => { new NumberValidator(precision, scale); };
action.Should().Throw<ArgumentException>();
}

[TestCase(1, 0, TestName = "scale = 0, precision > scale")]
[TestCase(2, 1, TestName = "scale > 0, precision > scale")]
[TestCase(2, 1, true, TestName = "scale > 0, precision > scale, onlyPositive = true")]
public void NumberValidator_CreateWithCorrectParams_Successes(int precision, int scale,
bool onlyPositive = false)
{
Action action = () => { new NumberValidator(precision, scale, onlyPositive); };
action.Should().NotThrow();
}

[Test]
public void Test()
public void IsValidNumber_CheckSameNumberTwoTimes_ResultsAreEqual()
{
var validator = new NumberValidator(17, 2, true);
var numberToCheck = "0.0";

var firstTimeCheck = validator.IsValidNumber(numberToCheck);
var secondTimeCheck = validator.IsValidNumber(numberToCheck);

firstTimeCheck.Should().Be(secondTimeCheck);
}

[TestCase(null, TestName = "value is null")]
[TestCase("", TestName = "value is empty")]
[TestCase(" ", TestName = "multiple whitespaces")]
[TestCase("a.sd", TestName = "value has letters")]
[TestCase("1.5.9", TestName = "more than one separator")]
[TestCase("3^2", TestName = "value has special symbols")]
public void IsValidNumber_CheckNonNumberValue_ReturnsFalse(string value)
{
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));

Assert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
Assert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0"));
Assert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
Assert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("00.00"));
Assert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("-0.00"));
Assert.IsTrue(new NumberValidator(17, 2, true).IsValidNumber("0.0"));
Assert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("+0.00"));
Assert.IsTrue(new NumberValidator(4, 2, true).IsValidNumber("+1.23"));
Assert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("+1.23"));
Assert.IsFalse(new NumberValidator(17, 2, true).IsValidNumber("0.000"));
Assert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("-1.23"));
Assert.IsFalse(new NumberValidator(3, 2, true).IsValidNumber("a.sd"));
var result = new NumberValidator(17, 2, true).IsValidNumber(value);

result.Should().BeFalse();
}

[TestCase("1234", 3, TestName = "integer and length > precision")]
[TestCase("0123", 3, TestName = "has leading zeros and total length > precision")]
[TestCase("-123", 3, TestName = "negative integer and length with sign > precision")]
[TestCase("-00123", 5, TestName = "negative integer with leading zero and length with sign > precision")]
[TestCase("0.1234", 3, TestName = "only decimal part and length > precision")]
[TestCase("1.234", 3, TestName = "total length > precision")]
[TestCase("1234.1", 4, TestName = "number is decimal and integer part length = precision")]
[TestCase("+1.23", 3, TestName = "has plus and length with sign > precision")]
[TestCase("-1.23", 3, TestName = "has minus and length with sign > precision")]
[TestCase("1.23", 3, 1, TestName = "decimal part length > scale")]
[TestCase("1.200", 4, 1, TestName = "has trailing zeros and decimal length > scale")]
[TestCase("-1.200", 4, 3, TestName = "negative, has trailing zeros and length without sign = precision")]
[TestCase(".123", 4, 3, TestName = "no digits before separator")]
[TestCase("2.", 4, 3, TestName = "no digits after separator")]
[TestCase("-1.3", 3, 2, true, TestName = "number is negative and onlyPositive = true")]
public void IsValidNumber_CheckNotValidNumber_ReturnsFalse(string value, int precision, int scale = 2,
bool onlyPositive = false)
{
var result = new NumberValidator(precision, scale, onlyPositive).IsValidNumber(value);

result.Should().BeFalse();
}

[TestCase("123", 3, TestName = "integer and length = precision")]
[TestCase("123", 4, TestName = "integer and length < precision")]
[TestCase("-12", 3, TestName = "negative integer and length with sign = precision")]
[TestCase("+1.23", 4, TestName = "has plus and length with sign = precision")]
[TestCase("-1.23", 4, TestName = "has minus and length with sign = precision")]
[TestCase("+1.03", 5, TestName = "has plus and length with sign < precision")]
[TestCase("-1.03", 5, TestName = "has minus and length with sign < precision")]
[TestCase("0.12", 3, TestName = "only decimal part and length = precision")]
[TestCase("0123", 4, 3, TestName = "has leading zeros and length = precision")]
[TestCase("1.200", 4, 3, TestName = "has trailing zeros and length = precision")]
[TestCase("0.1", 3, 1, TestName = "only decimal part and length < precision")]
[TestCase("1.23", 3, 2, TestName = "decimal part length = scale")]
[TestCase("1.23", 4, 3, TestName = "decimal part length < scale")]
[TestCase("-1.13", 4, 2, false, TestName = "negative and onlyPositive = false")]
[TestCase("+1.13", 4, 2, true, TestName = "has plus and onlyPositive = true")]
[TestCase("+1.13", 4, 2, false, TestName = "has plus and onlyPositive = false")]
public void IsValidNumber_CheckValidNumber_ReturnsTrue(string value, int precision, int scale = 2,
Copy link
Copy Markdown

@dmnovikova dmnovikova Nov 30, 2023

Choose a reason for hiding this comment

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

Тут и в соседних тестах получилось слишком много тесткейсов, из-за этого сложно отслеживать все ли случаи учтены и находить упавший — страдает читаемость кода
Большой подсказкой, что некоторые тесткейсы нужно разнести по разным тестам является схожее описание в TestName

bool onlyPositive = false)
{
var result = new NumberValidator(precision, scale, onlyPositive).IsValidNumber(value);

result.Should().BeTrue();
}
}

Comment thread
dmnovikova marked this conversation as resolved.
Expand All @@ -45,18 +117,12 @@ public NumberValidator(int precision, int scale = 0, bool onlyPositive = false)
if (precision <= 0)
throw new ArgumentException("precision must be a positive number");
if (scale < 0 || scale >= precision)
throw new ArgumentException("precision must be a non-negative number less or equal than precision");
throw new ArgumentException("scale must be a non-negative number less than precision");
numberRegex = new Regex(@"^([+-]?)(\d+)([.,](\d+))?$", RegexOptions.IgnoreCase);
}

public bool IsValidNumber(string value)
{
// Проверяем соответствие входного значения формату N(m,k), в соответствии с правилом,
Copy link
Copy Markdown

@dmnovikova dmnovikova Nov 30, 2023

Choose a reason for hiding this comment

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

а почему оставил комменты ниже?
названия у переменных вполне информативные, комментарии их только дублируют, создавая зрительный шум

// описанным в Формате описи документов, направляемых в налоговый орган в электронном виде по телекоммуникационным каналам связи:
// Формат числового значения указывается в виде N(m.к), где m – максимальное количество знаков в числе, включая знак (для отрицательного числа),
// целую и дробную часть числа без разделяющей десятичной точки, k – максимальное число знаков дробной части числа.
// Если число знаков дробной части числа равно 0 (т.е. число целое), то формат числового значения имеет вид N(m).

if (string.IsNullOrEmpty(value))
return false;

Expand All @@ -72,9 +138,7 @@ public bool IsValidNumber(string value)
if (intPart + fracPart > precision || fracPart > scale)
return false;

if (onlyPositive && match.Groups[1].Value == "-")
return false;
return true;
return !(onlyPositive && match.Groups[1].Value == "-");
}
}
}
28 changes: 15 additions & 13 deletions cs/HomeExercises/ObjectComparison.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,19 @@ namespace HomeExercises
public class ObjectComparison
{
[Test]
[Description("Проверка текущего царя")]
[Category("ToRefactor")]
public void CheckCurrentTsar()
public void GetCurrentTsar_CurrentTsarEqualsHistoricalTsar()
Comment thread
dmnovikova marked this conversation as resolved.
{
var actualTsar = TsarRegistry.GetCurrentTsar();

var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
new Person("Vasili III of Russia", 28, 170, 60, null));

// Перепишите код на использование Fluent Assertions.
Assert.AreEqual(actualTsar.Name, expectedTsar.Name);
Assert.AreEqual(actualTsar.Age, expectedTsar.Age);
Assert.AreEqual(actualTsar.Height, expectedTsar.Height);
Assert.AreEqual(actualTsar.Weight, expectedTsar.Weight);

Assert.AreEqual(expectedTsar.Parent!.Name, actualTsar.Parent!.Name);
Assert.AreEqual(expectedTsar.Parent.Age, actualTsar.Parent.Age);
Assert.AreEqual(expectedTsar.Parent.Height, actualTsar.Parent.Height);
Assert.AreEqual(expectedTsar.Parent.Parent, actualTsar.Parent.Parent);
actualTsar
.Should()
.BeEquivalentTo(expectedTsar, options =>
options.Excluding(property =>
property.SelectedMemberInfo.DeclaringType == typeof(Person) &&
property.SelectedMemberInfo.Name.Equals(nameof(Person.Id))));
}

[Test]
Expand All @@ -36,6 +30,14 @@ public void CheckCurrentTsar_WithCustomEquality()
new Person("Vasili III of Russia", 28, 170, 60, null));

// Какие недостатки у такого подхода?
/*
* 1) Если тест не прошел, не будет возможности понять, какие именно поля не равны - тест просто вернет false
* 2) При изменении класса Person необходимо будет изменять тест
* 3) Название теста не раскрывает смысла теста, не работает как спецификация
* 4) Проверку объектов на равенство лучше внести в класс
* 5) При наличии кольцевой ссылки кастомный тест уйдет в бесконечную рекурсию,
* а тест на FluentAssertions вернет false и укажет на наличие цикла
*/
Assert.True(AreEqual(actualTsar, expectedTsar));
}

Expand Down