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


Я задал аналогичный вопрос вчера, которое было подвергнуто критике, поскольку я написал тест без поддержки кода. Поэтому я вчера удалил вопрос, как он не был достаточно ясным. Я сейчас еще раз запостить с поддержки кода. Смотрите ниже тестовый код:

[TestFixture]
public class CustomerTest
{
    private IList<Product> _products;
    private Concor _concor;
    private Chestnut _chestnut;

    private IOfferCalculator _offerCalculator;

    [OneTimeSetUp]
    public void OneTimeSetUp()
    {
        _offerCalculator = new OfferCalculator();
    }

    [SetUp]
    public void Setup()
    {
        _concor = new Concor();
        _chestnut = new Chestnut();
        _products = new List<Product>();
        _products.Add(_concor);
        _products.Add(_chestnut);
    }

    [Test]
    public void Should_receive_a_for_concor()
    {
        var expectedConcor = new List<Product>();
        expectedConcor.Add(_concor);
        var gender = "F";
        var expenditure = 1500M;
        var Customer = new Customer(gender,expenditure);
        //Act
        Customer.AssignOffers(_offerCalculator, _products);
        //Assert
        CollectionAssert.AreEqual(Customer._assignedProducts, expectedConcor);
    }
}

Вот соответствующий код:

public class Concor : Product
    {
        public override bool Check(Customer customer)
        {
            if (customer._expenditure > 100 && customer._gender == "F")
            {
                return true;
            }
            else
            {
                return false;
            }
        }
    }

    public class Chestnut : Product
    {
        public override bool Check(Customer customer)
        {
            if (customer._expenditure >  100 && customer._gender =="M")
            {
                return true;
            }
            else
            {
                return false;
            }
        }
    }

    public abstract class Product
    {
        public abstract bool Check(Customer customer);
    }

    public interface IOfferCalculator
    {
        IEnumerable<Product> Calculate(Customer customer, IList<Product> products);
    }

    public class Customer
    {
        public string _gender { get; protected set; }
        public decimal _expenditure { get; protected set; }
        public IList<Product> _assignedProducts = new List<Product>();

        public Customer(string gender, decimal expenditure)
        {
            _gender = gender;
            _expenditure = expenditure;
        }

        public void AssignOffers(IOfferCalculator offerCalculator, IList<Product> products)
        {
            foreach (var product in offerCalculator.Calculate(this, products))
            {
                if (!_assignedProducts.Contains(product))
                {
                    _assignedProducts.Add(product);
                }
            }
        }
    }

    public class OfferCalculator : IOfferCalculator
    {
        public IEnumerable<Product> Calculate(Customer customer, IList<Product> products)
        {
            foreach (var product in products)
            {
                if (product.Check(customer))
                {
                    yield return product;
                }
            }
        }
    }

Я хотел бы некоторую обратную связь о качестве теста. Компании любят использовать БДД, однако я сознаю, что я утверждая, государства, а не поведение (утверждая, что я поверочных расчетов, а не оркестровки). Я также хотел бы некоторую обратную связь о моем использовании: OneTimeSetup атрибут и атрибут настройки. Также как бы проверить на Каштан? Добавить новый метод или использовать атрибут для тестов?



254
5
задан 27 января 2018 в 12:01 Источник Поделиться
Комментарии
1 ответ

Вы спрашиваете о тестах, но есть несколько проблем в коде, которые должны быть решены перед началом тестирования.



public abstract class Product
{
public abstract bool Check(Customer customer);
}

Вы сделали Product абстрактный класс, но это и не нужно. Он не обеспечивает реализации по умолчанию на все так простой интерфейс будет достаточно.

Существует, однако, одна проблема с ним. В Check метод. Здесь ничего не говорится о том, что проверяется.


customer._expenditure >  100 && customer._gender =="M"

Его реализация оценивается выше состояние, но нет никакой информации о нем в имя метода, чтобы никто не мог сказать, что он проверяет.

Поверх него он даже использует жестко Magic-номер 100. Это должно быть постоянным.

Можно также положить его внутри if/else что не надо. Вы можете просто return результат.



public void AssignOffers(IOfferCalculator offerCalculator, IList<Product> products)
{
foreach (var product in offerCalculator.Calculate(this, products))
{
if (!_assignedProducts.Contains(product))
{
_assignedProducts.Add(product);
}
}
}

Это метод тестирования, но это трудно проверить, потому что это делает много: он фильтрует список товара, он проверяет, является ли она назначена, и он добавляет его в другой список. Для меня это три разных обязанностей. Вы должны как минимум пройти уже отфильтрованный список продуктов.

Метод также не информирует меня, что все будет фильтроваться на всех. Он говорит AssignOffers но вместо того, чтобы ассинг предлагает он вычисляет что-то и он работает с продукции, что добавляет еще один список, но он не назначает ничего. Все сообщают об этом методе и она полна сюрпризов.


Сказав, что я найду все ваши тесты не имеют смысла, поскольку реализация смысла тоже. Код сбивает с толку и делает совершенно разные вещи, что имена посоветую. Писать тесты для такого кода не имеет смысла.

Вы должны сначала зафиксировать основной код, так что понятно, что она делает, так что вы знаете, что вы испытываете. Попробуйте подобрать имена, которые четко сообщить цель и сделать ровно только что.

Например, если вы Calculate что-то, то пусть этот метод возвращает результат этого вычисления, основанные на несколько параметров, как это возможно. Прохождение всех объектов, где требуется только два свойства-это код запах.



public IEnumerable<Product> Calculate(Customer customer, IList<Product> products)
{
foreach (var product in products)
{
if (product.Check(customer))
{
yield return product;
}
}
}

Но подождите минуту! Давайте взглянем на то, что Calculate на самом деле... на мой (не) удивлению, он не рассчитывает что-нибудь, но вместо этого он проверяет продукты против клиента!!! Насколько запутано это?


Это такой бардак, что вы можете бросить все эти тесты подальше и сначала полностью переписать код, то писать новые тесты.

7
ответ дан 27 января 2018 в 01:01 Источник Поделиться