Sales tax calculator, rejected for being not OOP

I applied for an Application Developer position. They require all their applicants to complete 1 of 3 programming assignments. I picked one for sales tax calculation. It was quite simple.


Write a program the feeds in prices. If the item is a Book, Food or
Medical product there is no tax, if not there is a 10% tax (other
item). If the item is an import there is a 5% tax (it's possible to
have an import and "other item" type of item with 15% tax (10 + 5)).
The program needs to print receipts like this...

• Book: 9.12 (2 @ 4.56)

• Imported dog food: 6.52

• Perfume: 6.23

• Sales Taxes: 1.65

• Total: 21.87

You can see in this case the application can "group" items and provide tax calculation including total tax.

To describe it my application had a menu the continuing prompting the user with information until they broke from the menu. The product information was placed into a List. A lambda expression is written against the list to group and get the duplicate items. An if statement nested within a foreach statement fires the tax calculation to print out the receipts.

```
namespace Cash
{

public class product
{
public string prod_name { get; set; }
public decimal prod_price { get; set; }
public bool is_import { get; set; }
public bool is_other { get; set; }
}

class Program
{

static void Main(string[] args)
{
Program prog = new Program();
prog.menu();
}

public void menu()
{
string men1;
string prod_name=null;
bool is_import;
bool is_other;
decimal prod_price;

var prod_list = new List();

while (true)
{
Console.WriteLine("Sales Tax");
Console.WriteLine();
Console.WriteLine("Please Select the Item Type");
Console.WriteLine("1

I guess what they were looking for was methods on classes. Also I recommend you download resharper and see what it says about your naming conventions as they are non standard.

A 'Classic OO' as I call it approach might look a bit like

public class Product
    {
        public string Name { get; private set; }
        public decimal BasePrice { get; private set; }
        public bool IsImport { get; private set; }

        /// 
        /// enum of book, food etc
        /// 
        public ItemType Type { get; private set; }

        /// 
        /// constructor allows you to set properties
        public Product(string name, decimal basePrice, bool isImport, ItemType Type)
        {
        }

        /// 
        /// business logic to work out sale price from private fields
        /// 
        /// 
        public decimal GetSalePrice()
        {
            //do calculation
        }
    }

Also, as another commenter notes. putting some inheritance in wouldn't hurt even though its not really required. The point of these interview tests is to demonstrate you know about stuff. Its assumed you will be able to solve the problem.

As a guide, I always include the following :

• Unit Tests - cannot stress this enough! Names match tests cases ie.

givenXThenY (TDD)

• Classes with names matching business terms (DDD)

• Methods on those classes (OO)

• Service classes with business logic (SoA)

• Interfaces for EVERYTHING (more OO/DI) also shows inheritance

knowledge*

• injection of services into constructors via interfaces (DI)

• ViewModel objects or some other separation of display (MVVM)

• Repository object (repository patter/separation of datalayer)

• At least one use of Generic Collections

• At least one use of Linq

• XML Comment everything (code readablity)

• Method Names which describe what they do (Clean Coding)

• More than one project per solution (not including test project)

• Name projects like name spaces ie YourCompany.Example5.Models

other stuff you can throw in if you think the question is angling for it

• Recursion

• abstract classes/ more inheritance

• the modulus operator % some tests angle for this, shows you know

maths?

• async method and Task, multithreading

*When I say EVERYTHING I mean Services AND Models, really you don't need both, but some people like one and some the other. A lot of these things are contradictory or double up on patterns. The point is not writing good code, its demonstrating you know about lots of patterns and coding practices etc

StackExchange Code Review Q#88528, answer score: 46