SalesTax problem (C# version)

One year ago I published an F# solution of the same task and there is an old C# solution. But I think it's a simple task and require a simple solution.

What do you think?

```
using System;
using System.Collections.Generic;

namespace SalesTaxes
{
class Program
{
static void Main(string[] args)
{
List 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 getItemsList()
{
List lstItems = new List();
//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; }

A couple of small suggestions.

Var:
You spend a lot of time redeclaring variable types when they are already well established by your code. For example:

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

I know there can be some debate about whether var is preferred or not, but I can say I work in a shop that currently uses it heavily and it makes refactoring a dream. Just changing the Foreach to:

foreach(var item in itemList)

would make refactoring much easier later. This way it will figure out the type for you at compile time, meaning itemList can be an ienumerable of anything as would still be valid in that case.

Console.WriteLine:
There is already a string format overload for Console.Writeline. The following lines are equivalent:

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

Collection Construction:
You can use simpler syntax to construct your collection in getItemsList() making your entire signature for the function something closer to this:

private static IEnumerable getItemsList()
    {
        return new List
        {
            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
            }
        };
    }

Return Type List:
For what you use the return value for, in getItemsList, it doesn't need to be returned as a List. It could be an IList or IEnumerable without any ill effects. It's not a huge deal in your case as it's private, but around where I work we generally try to return collections under there interfaces. I know resharper will flag this as well.

Naming:
The enumeration ProductTypes values and the getItemsList method do not follow C# standards for naming. It really should be GetItemsList and Food/Book/Medical/Other.

IsExempt
Why is it:

return (int)Type < 4;

instead of:

return Type != ProductType.Other;

They mean the same thing, but one is a bit more obvious as to it's intentions.

Taxes Getter:
This is more of a soft suggestion, but this getter is a bit convoluted to read. I had to dig through and add some spacing to figure out what it was doing and even then I misplaced a paren the first time and got the wrong solution. I understand what you are doing with this type of code but it's honestly something that will be more of a headache than a help later as it is not immediately obvious what it is doing unless you dig in. Maybe simplifying out the ternary statements into getters for those values, or methods if you prefer, would be a better solution as it would vastly simplify the amount of parenthesis and make the statement a bit easier on the eyes. Example:

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;
    }

This seems much easier to read at a glance.

Writelines
Also worth noting that you swap between the string.format method of writing variables out to the string concatenation method in a couple of places:

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

Best to stick with a style when you start. I prefer the string format approach, using the appropriate Console.WriteLine overload, as it hedges away from doing string + string. In this case it's really not more efficient, but it's a bad to get into the habit of string + string, as in non-trivial usages of concatenation it's inefficient. When I find yourself using string + string it's usually a tip off that there is a better way to be doing it (StringBuilder, String.Format, etc). Once again, no actual gain from this in your instance, just a good habit to build upon.

Overall:
The solution itself looks pretty good, though I'll admit I didn't run it through too much in the way of testing. I would just suggest those small style changes. Hope that helps.

StackExchange Code Review Q#110474, answer score: 12