Гипотетический SalesTax вызов


У меня есть небольшое решение следующую гипотетическую проблему:

Основной налог с продаж применяется в размере 10% на все товары, за исключением книг, продуктов питания и медицинских изделий, которые освобождаются. Ввозная пошлина является дополнительный налог с продаж, применимых на все импортные товары в размере 5%, без исключений.

При покупке вещей я получаю квитанцию, где перечислены имена всех товаров и их цена (включая налоги), заканчивая общей стоимость товаров и суммы налогов уплачены. В правила округления для налога, что налоговая ставка в размере Н%, полка цена P содержит (НП/100, округленный до ближайшего 0.05) количество налог с продаж. Написать приложение, которое выводит подробности получения для этих корзин.

Ввода:

  • Вход 1: Книга 1 в 12.49 1 музыкальный компакт-диск в 14.99 1 шоколадку на 0.85
  • Вход 2: 1 импортированная коробка конфет в 10.00 1 импортные духи у 47.50
  • Вход 3: 1 импортные духи у 27.99 1 флакон духов на 18.99 1 пачку таблеток от головной боли в размере 9,75 1 коробка импортного конфеты в 11.25

Выход

  • Выход 1: Книга 1 : 12.49 1 музыкальный компакт-диск: 16.49 1 шоколадку: 0.85 налогов: 1.50 Итого: 29.83
  • Выхода 2: 1 импортированная коробка конфет: 10.50 1 импортные бутылки дух: 54.65 налогов: 7.65 Итого: 65.15
  • Выхода 3: 1 импортные бутылки дух: 32.19 1 флакон духов: 20.89 1 пачку таблеток от головной боли: 9.75 1 импортную коробку конфет: 11.85 налогов: 6.70 общая: 74.68

Меня интересуют отзывы о Использование шаблонов проектирования, существуют ли какие-либо вопросы расширяемости, как сплошной код и стиль/структура модульных тестов. Я бы очень рады услышать конструктивную критику.

Полное решение доступно здесь

Это является частью основного решения:

public class SalesItem : ISalesItem
{
    private string _name;
    private decimal _price;

    public SalesItem(string name, decimal price)
    {
        #region Parameter Checking
        if (String.IsNullOrWhiteSpace(name))
            throw new ArgumentException("name");
        if(price < 0)
            throw new ArgumentException("price");
        #endregion

        this._name = name;
        this._price = price;
    }

    #region ISalesItem Members

    public string Name
    {
        get { return this._name; }
    }

    public decimal GetPrice()
    {
        return this._price;
    }

    public decimal GetSalesTax()
    {
        return 0.0M;
    }

    public decimal GetTotal()
    {
        return this.GetPrice() + this.GetSalesTax();
    }

    #endregion
}

public class SalesItemTaxDecorator: ISalesItem
{
    protected ISalesItem _decoratedSalesItem;
    protected ITax _salesTax;

    public SalesItemTaxDecorator(ISalesItem salesItem, ITax salesTax)
    {
        this._decoratedSalesItem = salesItem;
        this._salesTax = salesTax;
    }

    #region ISalesItem Members

    public string Name
    {
        get { return _decoratedSalesItem.Name; }
    }

    public virtual decimal GetSalesTax()
    {
        return this._decoratedSalesItem.GetSalesTax() + _salesTax.CalculateTax(this._decoratedSalesItem.GetPrice());
    }

    public virtual decimal GetPrice()
    {
        return this._decoratedSalesItem.GetPrice();
    }

    public virtual decimal GetTotal()
    {
        return this.GetPrice() + this.GetSalesTax();
    }

    #endregion
}

public class Tax : ITax
{
    private decimal _rate;
    private IRounding _rounding;

    public Tax(decimal rate, IRounding rounding)
    {
        this._rate = rate;
        this._rounding = rounding;
    }

    #region ISalesTax Members

    public decimal Rate
    {
        get { return this._rate; }
    }

    public IRounding Rounding
    {
        get { return this._rounding; }
    }

    public virtual decimal CalculateTax(decimal itemPrice)
    {
        decimal tempTax = itemPrice * this.Rate;
        decimal roundedTax = Rounding.Round(tempTax);
        return roundedTax;
    }

    #endregion
}

public static class SalesItemFactory
{
    private static readonly Rounding ROUNDING = new Rounding(0.05M);
    private static readonly ITax BASICTAX = new Tax(0.1M, ROUNDING);
    private static readonly ITax IMPORTTAX = new Tax(0.05M, ROUNDING);

    private static readonly Dictionary<ItemType, ITax> itemTaxLookup = new Dictionary<ItemType, ITax>()
    {
        { ItemType.Basic, BASICTAX },
        { ItemType.Import, IMPORTTAX }
    };

    public static ISalesItem GetSalesItem(string name, decimal price, ItemType itemType)
    {
        ISalesItem item = new SalesItem(name, price);

        foreach (int flag in Enum.GetValues(typeof(ItemType)))
        {
            if ((flag & (int)itemType) == flag)
            {
                item = (ISalesItem)Activator.CreateInstance(typeof(SalesItemTaxDecorator), new object[] { item, itemTaxLookup[(ItemType)flag] });
            }
        }

        return item;
    }

    public static ISalesItem GetSalesItem(string name, decimal price)
    {
        return new SalesItem(name, price);
    }
}


Комментарии
1 ответ

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

1) http://en.wikipedia.org/wiki/Overengineering. Просмотрел решение и нашли абсолютно никакого смысла в таком количестве интерфейсов. Я бы наверняка избавиться от ITax и IRounding, потому что есть только одна реализация для каждого из них и нет никакого смысла изолировать их в юнит-тестирования, потому что там слишком мало функций, чтобы изолировать. Не делайте это в такой сложный путь, пока вам действительно нужна такая сложная модель. У вас будет время, чтобы сделать вещи хуже позже :)

2) лично я ненавижу 4-линия регионах - они не помогают вообще, как для меня.

3) Если у вас есть завод по salesItems тогда почему вы не хотите сделать класс SalesItem скрыты внутри фабрики класса.

public static class SalesItemFactory
{
class SalesItem : ISalesItem { ... }
class SalesItemTaxDecorator : ISalesItem { ... }

...
}

Если вы действительно хотите, чтобы юнит-тест их, то по крайней мере сделать их внутренних и использовать InternalsVisibleTo. Или лучше даже не испытывать их, заводские испытания!

4) пункт = (ISalesItem)активатор.Метод createinstance(typeof на(SalesItemTaxDecorator), новый объект[] { пункт, itemTaxLookup[(Пункттип флаг)] });

Почему активатор на все?

5) той же строке, как и в предыдущем пункте. Почему вы присваиваете значение элемента и продолжения итераций? Просто вернуть его! И для того, чтобы сохранить вашей логике вы должны вернуть коллекцию, возвращаемую перечисление.GetValues.

6) я хотел бы удалить "защищены" и "виртуальные" вещи из своего декоратора и заменить их 'личное' и 'запечатанных' вместо этого. В противном случае вы даете возможность украсить декоратора, но это странно. Слишком много расширяемости

7) Давайте юнит-тесты :). https://github.com/manwood/SalesTax/tree/master/Manwood.SalesTax.Tests/Stubs. Не пиши корешки, использовать например вместо RhinoMock. Они делают жизнь проще по многим причинам: вам не придется писать свои корешки, вы не увидите корешки, а ищет наследников ITax, они позволяют писать более точные юнит-тесты (ваш юнит-тесты не так хорошо, см. Следующий пункт).

8) https://github.com/manwood/SalesTax/blob/master/Manwood.SalesTax.Tests/SalesItemTest.cs.

[TestClass]
public class SalesItemTest
{
[TestMethod] public void SalesItemGetPriceTest() { }
[TestMethod] public void SalesItemGetSalesTaxTest() { }
[TestMethod] public void SalesItemGetTotalTest() { }
}

Удвоить ваши имена слишком много в testMethods. PriceTest(), SalesTaxTest(), GetTotalTest() было бы лучше имена.

9) https://github.com/manwood/SalesTax/blob/master/Manwood.SalesTax.Tests/SalesItemTaxDecoratorTest.cs

    [TestMethod]
public void SalesItemTaxDecoratorTotalTest()
{
SalesItemTaxDecorator taxDecorator = new SalesItemTaxDecorator(new SalesItemStub(10M, 0M), new TaxStub(0.1M));
decimal total = taxDecorator.GetTotal();
Assert.AreEqual(11M, total);
}

Это неверный тест. Что вы проверяете, является ли он вернется 11 если цена 10 и налог 0.1. Что вы должны проверить-это а) налог был рассчитан по стоимости (это означает, что ITax.CalculateTax был вызван с параметром 10) Б) результат берется из налогов был добавлен в 10 и вернулись. RhinoMocks поможет использовать assert а) заявление, и вы должны утверждать б) самостоятельно таким же образом, Вы делаете это прямо сейчас. Этот тест не должен иметь 0.1 константы, в противном случае вы тестируете налогового класса (даже если это стаб).

10) 10-й пункт, я хочу спать.

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

10
ответ дан 1 февраля 2011 в 11:02 Источник Поделиться