Skip to content

Яценко Ирина #235

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
lholypeachy wants to merge 9 commits into
base: master
Choose a base branch
from
Open

Яценко Ирина #235

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
2c1d986
Сравнение объектов
lholypeachy Nov 27, 2023
3938cbe
Рефакторинг тестов
lholypeachy Nov 27, 2023
d229179
Исправление ошибок
lholypeachy Nov 28, 2023
09348a8
Создание TestCaseSource
lholypeachy Nov 28, 2023
5f3e0e4
Рефакторинг кода
lholypeachy Nov 28, 2023
6eb6ff0
Исправление ошибок и добавление новых тестовых случаев
lholypeachy Nov 29, 2023
2146ede
Рефакторинг и добавление новых тестовых случаев
lholypeachy Nov 29, 2023
8832ca8
Разделение классов по файлам
lholypeachy Nov 30, 2023
def3bbb
Скорректировала название файла
lholypeachy Nov 30, 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
95 changes: 35 additions & 60 deletions cs/HomeExercises/NumberValidatorTests.cs
View file
Open in desktop
Copy link

@pakapik pakapik Nov 28, 2023

Choose a reason for hiding this comment

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

Круто, что коммиты логически разбиты

Original file line number Diff line number Diff line change
@@ -1,132 +1,107 @@
using System;
using System.Text.RegularExpressions;
using FluentAssertions;
using NUnit.Framework;

namespace HomeExercises
{
public class NumberValidatorTests
Copy link

@pakapik pakapik Nov 28, 2023

Choose a reason for hiding this comment

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

using FluentAssertions; не используется. Обычно ide сама убирает неиспользуемые неймспейсы, но не всегда.

Copy link

@pakapik pakapik Nov 29, 2023

Choose a reason for hiding this comment

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

Ранее об этом не говорил, но класс тоже можно пометить атрибутом TestFixture, причем туда же можно по желанию так же впихнуть TestOf: [TestFixture(TestOf = typeof(NumberValidator))]

{
[Test]
public void NumberValidator_WhenPassNegativePrecision_ShouldThrowsArgumentException()
public void NumberValidatorCtor_WhenPassNegativePrecision_ShouldThrowsArgumentException()
Copy link

@pakapik pakapik Nov 28, 2023

Choose a reason for hiding this comment

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

Оставлю ссылку на этот комментарий, и ещё раз скажу, что тесты, которые выкидывают исключение, можно объединить в один, указав для них TestCaseSource

{
TestDelegate testDelegate = () => new NumberValidator(-1, 2, true);

Assert.Throws<ArgumentException>(testDelegate);
}

[Test]
public void NumberValidator_WhenPassNegativeScale_ShouldThrowsArgumentException()
public void NumberValidatorCtor_WhenPassNegativeScale_ShouldThrowsArgumentException()
{
TestDelegate testDelegate = () => new NumberValidator(1, -2);

Assert.Throws<ArgumentException>(testDelegate);
}

[Test]
public void NumberValidator_WhenPassValidArguments_ShouldDoesNotThrows()
public void NumberValidatorCtor_WhenPassValidArguments_ShouldDoesNotThrows()
{
TestDelegate testDelegate = () => new NumberValidator(1, 0, true);

Assert.DoesNotThrow(testDelegate);
Copy link

@pakapik pakapik Nov 29, 2023

Choose a reason for hiding this comment

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

Тут тоже можно до одной строки упростить.

}

[Test]
public void NumberValidator_WhenPrecisionIsEqualToTheScale_ShouldReturnFalse()
public void NumberValidatorCtor_WhenPrecisionIsEqualToTheScale_ShouldThrowsArgumentException()
{
TestDelegate testDelegate = () => new NumberValidator(2, 2, true);

Assert.Throws<ArgumentException>(testDelegate);
}

[Test]
public void IsValidNumber_WhenPassOnlyPositiveIsFalseButNumbersDoesNotHaveNegativeSign_ShouldReturnTrue()
public void IsValidNumberTest(int precision, int scale, bool onlyPositive,
string number, bool expectedResult)
{
var validator = new NumberValidator(17, 2);
var validator = new NumberValidator(precision, scale, onlyPositive);

var numberIsValid = validator.IsValidNumber("1.0");
var actualResult = validator.IsValidNumber(number);

Assert.True(numberIsValid);
Assert.AreEqual(expectedResult, actualResult);
}

[Test]
public void IsValidNumber_WhenLettersInsteadOfNumber_ShouldReturnFalse()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenFractionalPartIsMissing_ShouldReturnTrue()
{
var validator = new NumberValidator(3, 2, true);

var numberIsValid = validator.IsValidNumber("a.sd");

Assert.IsFalse(numberIsValid);
IsValidNumberTest(17,2,true,"0", true);
Copy link

@pakapik pakapik Nov 28, 2023

Choose a reason for hiding this comment

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

Тут тоже нужно подумать в сторону TestCaseSource. Вызывать внутри теста другой тест - плохая практика.
Под обобщением создания корректного валидатора я подразумевал немного иное. Подумай, как можно получать в методе / группе методов корректный валидатор, не вызывая непосредственно конструктор?

Но опустим пока этот момент. Сейчас тебе надо сделать так, чтоб все твои 8 тестов, о которых мы говорили, превратились в один, для которых бы использовался один или несколько TestCaseSource.

}

[Test]
public void IsValidNumber_WhenSymbolsInsteadOfNumber_ShouldReturnFalse()
{
var validator = new NumberValidator(3, 2, true);

var numberIsValid = validator.IsValidNumber("2.!");

Assert.IsFalse(numberIsValid);
[Test]
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenLettersInsteadOfNumber_ShouldReturnFalse()
{
IsValidNumberTest(3, 2, true, "a.sd", false);
}

[Test]
public void IsValidNumber_WhenFractionalPartIsMissing_ShouldReturnTrue()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenSymbolsInsteadOfNumber_ShouldReturnFalse()
{
var validator = new NumberValidator(17, 2, true);

var numberIsValid = validator.IsValidNumber("0");

Assert.IsTrue(numberIsValid);
IsValidNumberTest(3, 2, true, "2.!", false);
}

[Test]
public void IsValidNumber_WhenNumberIsNull_ShouldReturnFalse()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenNumberIsNull_ShouldReturnFalse()
{
var validator = new NumberValidator(17, 2, true);

var numberIsValid = validator.IsValidNumber(null!);

Assert.IsFalse(numberIsValid);
IsValidNumberTest(17,2,true, null!, false);
}

[Test]
public void IsValidNumber_WhenPassNumberIsEmpty_ShouldReturnFalse()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenPassNumberIsEmpty_ShouldReturnFalse()
{
var validator = new NumberValidator(3, 2, true);

var numberIsValid = validator.IsValidNumber("");

Assert.IsFalse(numberIsValid);
IsValidNumberTest(3,2,true,"", false);
}

[Test]
public void IsValidNumber_WhenIntPartWithNegativeSignMoreThanPrecision_ShouldReturnFalse()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenIntPartWithNegativeSignMoreThanPrecision_ShouldReturnFalse()
{
var validator = new NumberValidator(3, 2, true);

var numberIsValid = validator.IsValidNumber("-0.00");

Assert.IsFalse(numberIsValid);
IsValidNumberTest(3,2,true,"-0.00", false);
}

[Test]
public void IsValidNumber_WhenIntPartWithPositiveSignMoreThanPrecision_ShouldReturnFalse()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenIntPartWithPositiveSignMoreThanPrecision_ShouldReturnFalse()
{
var validator = new NumberValidator(3, 2, true);

var numberIsValid = validator.IsValidNumber("+1.23");

Assert.IsFalse(numberIsValid);
IsValidNumberTest(3,2,true,"+1.23", false);
}

[Test]
public void IsValidNumber_WhenFractionalPartMoreThanScale_ShouldReturnFalse()
[TestOf(nameof(NumberValidator.IsValidNumber))]
public void WhenFractionalPartMoreThanScale_ShouldReturnFalse()
{
var validator = new NumberValidator(17, 2, true);

var numberIsValid = validator.IsValidNumber("0.000");

Assert.IsFalse(numberIsValid);
IsValidNumberTest(17,2,true, "0.000", false);
}
}

Expand Down
41 changes: 20 additions & 21 deletions cs/HomeExercises/ObjectComparison.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,21 @@ namespace HomeExercises
{
public class ObjectComparison
{
[Test]
[Description("Проверка текущего царя")]
[Category("ToRefactor")]
public void CheckCurrentTsar()
{
var actualTsar = TsarRegistry.GetCurrentTsar();
[Test]
[Description("Проверка текущего царя")]
[Category("ToRefactor")]
public void CheckCurrentTsar()
{
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));
var expectedTsar = new Person("Ivan IV The Terrible", 54, 170, 70,
new Person("Vasili III of Russia", 28, 170, 60, null));

// Перепишите код на использование Fluent Assertions.
actualTsar.Should()
.BeEquivalentTo(expectedTsar, x => x
.Excluding(x => x.Id)
.Excluding(x => x.Parent!.Id));
}
actualTsar.Should()
.BeEquivalentTo(expectedTsar, x => x
.Excluding(x => x.Id)
.Excluding(x => x.Parent!.Id));
}

[Test]
[Description("Альтернативное решение. Какие у него недостатки?")]
Expand All @@ -31,13 +30,13 @@ public void CheckCurrentTsar_WithCustomEquality()
new Person("Vasili III of Russia", 28, 170, 60, null));

// Какие недостатки у такого подхода?
//Данный подход делает класс труднорасширяемым, ведь при добавлении новых полей в класс Person
//нужно переписывать и метод сравнения для всех этих полей, так же новые поля cможет добавить
//другой разработчик, который не знает о таком методе сравнения, что приведет к новым, неотловоенным ошибкам -
//новые поля не будут сравниваться.
//В моем решении (CheckCurrentTsar), такой ошибки не возникнет и класс Person сможет без ошибок расширяться
//при условии, что сравниваться будут все поля, кроме Id (так же и их Parent), так как код написан не перебиранием
//всех полей для сравнения, а сравнением объекта в целом с исключением его Id.
// Данный подход делает класс труднорасширяемым, ведь при добавлении новых полей в класс Person
// нужно переписывать и метод сравнения для всех этих полей, так же новые поля cможет добавить
// другой разработчик, который не знает о таком методе сравнения, что приведет к новым, неотловоенным ошибкам -
// новые поля не будут сравниваться.
// В моем решении (CheckCurrentTsar), такой ошибки не возникнет и класс Person сможет без ошибок расширяться
// при условии, что сравниваться будут все поля, кроме Id (так же и их Parent), так как код написан не перебиранием
// всех полей для сравнения, а сравнением объекта в целом с исключением его Id.
Assert.True(AreEqual(actualTsar, expectedTsar));
}

Expand Down