Проблема с SalesTax (версия C #)

Год назад я опубликовал решение F # той же задачи, и есть старое решение C # . Но я думаю, что это простая задача и требует простого решения.

Как вы думаете?

using System;
using System.Collections.Generic;

namespace SalesTaxes
{
    class Program
    {
        static void Main(string[] args)
        {
            List<ShoppingCartItem> itemList = getItemsList();

            decimal salestaxes = 0.00m;
            decimal totalprice = 0.00m;

            foreach (ShoppingCartItem item in itemList)
            {
                salestaxes += item.Taxes * item.Quantity;
                totalprice += item.Item.Price * item.Quantity;
                Console.WriteLine(string.Format("{0} {1} : {2}", item.Quantity, item.Item.Name, (item.Item.Price + item.Taxes) * item.Quantity));
            }
            totalprice += salestaxes;
            Console.WriteLine("Sales Taxes : " + salestaxes);
            Console.WriteLine("Total : " + totalprice);
            Console.ReadLine();
        }

        private static List<ShoppingCartItem> getItemsList()
        {
            List<ShoppingCartItem> lstItems = new List<ShoppingCartItem>();
            //input 1

            lstItems.Add(new ShoppingCartItem { Item = new Product { Name = "Book", Price = 12.49m, Type = Product.ProductType.book, IsImport = false }, Quantity = 1 });
            lstItems.Add(new ShoppingCartItem { Item = new Product { Name = "music CD", Price = 14.99m, Type = Product.ProductType.other, IsImport = false }, Quantity = 1 });
            lstItems.Add(new ShoppingCartItem { Item = new Product { Name = "chocolate bar", Price = 0.85m, Type = Product.ProductType.food, IsImport = false }, Quantity = 1 });

            return lstItems;
        }
    }

    public class Product
    {
        public enum ProductType
        {
            food = 1,
            book = 2,
            medical = 3,
            other = 4
        };

        public string Name { get; set; }        
        public decimal Price { get; set; }
        public ProductType Type { get; set; }
        public bool IsImport { get; set; }
        public bool IsExempt
        {
            get
            {
                return (int)Type < 4;
            }
        }
    }    

    public class ShoppingCartItem
    {
        const decimal TaxRate = 0.1m;
        const decimal ImpTaxRate = 0.05m;
        public Product Item { get; set; }
        public int Quantity { get; set; }
        public decimal Taxes
        {
            get
            {
                return decimal.Ceiling(Item.Price * ((Item.IsExempt ? 0 : TaxRate) + (Item.IsImport ? ImpTaxRate : 0)) * 20) / 20;
            }
        }
    }
}
8 голосов | спросил Alexan 11 32015vEurope/Moscow11bEurope/MoscowWed, 11 Nov 2015 21:13:05 +0300 2015, 21:13:05

1 ответ


12

Несколько небольших предложений.

Var: Вы тратите много времени на повторное использование переменных типов, когда они уже хорошо установлены вашим кодом. Например:

decimal totalprice = 0.00m;
foreach (ShoppingCartItem item in itemList){...}
List<ShoppingCartItem> lstItems = new List<ShoppingCartItem>();

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

foreach(var item in itemList)

сделает рефакторинг намного проще позже. Таким образом, он будет определять тип для вас во время компиляции, что означает, что itemList может быть ienumerable чего-либо, что было бы все равно в этом случае.

Console.WriteLine: Для Console.Writeline уже существует перегрузка формата строки. Следующие строки эквивалентны:

Console.WriteLine(string.Format("{0} ", x));
Console.WriteLine("{0}",x);

Конструкция коллекции: Вы можете использовать более простой синтаксис для создания вашей коллекции в getItemsList (), чтобы ваша целая подпись для функции была ближе к этому:

    private static IEnumerable<ShoppingCartItem> getItemsList()
    {
        return new List<ShoppingCartItem>
        {
            new ShoppingCartItem
            {
                Item = new Product {Name = "Book", Price = 12.49m, Type = Product.ProductType.book, IsImport = false},
                Quantity = 1
            },
            new ShoppingCartItem
            {
                Item = new Product{Name = "music CD",Price = 14.99m,Type = Product.ProductType.other,IsImport = false},
                Quantity = 1
            },
            new ShoppingCartItem
            {
                Item = new Product{Name = "chocolate bar",Price = 0.85m,Type = Product.ProductType.food,IsImport = false},
                Quantity = 1
            }
        };
    }

Список возвращаемых типов: Для того, что вы используете для возвращаемого значения, в getItemsList его не нужно возвращать в виде списка. Это может быть IList или IEnumerable без каких-либо негативных последствий. Это не огромная сделка в вашем случае, поскольку она закрыта, но вокруг, где я работаю, мы обычно пытаемся вернуть коллекции под этими интерфейсами. Я знаю, что resharper также отметит это.

Нейминг: Значения ProductTypes перечисления и метод getItemsList не соответствуют стандартам C # для именования. Это действительно должно быть GetItemsList и Food /Book /Medical /Other.

IsExempt Почему это:

return (int)Type < 4;

вместо:

return Type != ProductType.Other;

Они означают одно и то же, но один из них более очевиден в отношении его намерений.

Налоги Getter: Это скорее мягкое предложение, но этот геттер немного запутан для чтения. Мне пришлось прорываться и добавить промежуток времени, чтобы понять, что он делает, и даже тогда я потерял место в первый раз и получил неправильное решение. Я понимаю, что вы делаете с этим типом кода, но это честно то, что будет скорее головной болью, чем помощью позже, так как не сразу видно, что она делает, если вы не заходите. Может быть, упростить тройные заявления в getters для эти значения или методы, если вы предпочитаете, были бы лучшим решением, поскольку это значительно упростило бы количество скобок и сделало бы заявление немного легче на глазах. Пример:

public decimal Taxes
    {
        get
        {
            return decimal.Ceiling( Item.Price *( CalculateTaxRate() + CalculateImportRate()) * 20) / 20;
        }
    }

    private decimal CalculateTaxRate()
    {
        return Item.IsExempt
            ? 0
            : TaxRate;
    }

    private decimal CalculateImportRate()
    {
        return Item.IsImport
            ? ImpTaxRate
            : 0;
    }

Это кажется намного легче читать с первого взгляда.

Writelines Также стоит отметить, что вы меняете между методом string.format записи переменных в метод конкатенации строк в нескольких местах:

Console.WriteLine(string.Format("{0} {1} : {2}", item.Quantity, item.Item.Name, (item.Item.Price + item.Taxes) * item.Quantity));
Console.WriteLine("Sales Taxes : " + salestaxes);

Лучше придерживаться стиля, когда вы начинаете. Я предпочитаю подход с строковым форматом, используя соответствующую перегрузку Console.WriteLine, поскольку она хеджирует от строки string +. В этом случае это действительно не более эффективно, но плохо входить в привычку string + string, так как в нетривиальных применениях конкатенации это неэффективно. Когда я нахожу, что использую строку string +, обычно это подсказка, что есть лучший способ сделать это (StringBuilder, String.Format и т. Д.). Еще раз, никакой реальной выгоды от этого в вашем случае, просто хорошая привычка, чтобы опираться.

В целом: Решение само по себе выглядит довольно неплохо, хотя я соглашусь, что я не использовал его слишком много для тестирования. Я бы просто предложил эти небольшие изменения стиля. Надеюсь, что это поможет.

ответил BlindGarret 11 32015vEurope/Moscow11bEurope/MoscowWed, 11 Nov 2015 23:26:50 +0300 2015, 23:26:50

Похожие вопросы

Популярные теги

security × 330linux × 316macos × 2827 × 268performance × 244command-line × 241sql-server × 235joomla-3.x × 222java × 189c++ × 186windows × 180cisco × 168bash × 158c# × 142gmail × 139arduino-uno × 139javascript × 134ssh × 133seo × 132mysql × 132