0
\$\begingroup\$

Now I have task very similar to this:

Job is a group of print items. For example, a job can be a run of business cards, envelopes, and letterhead together.

Some items qualify as being sales tax free, whereas, by default, others are not. Sales tax is 7%.

Also applies a margin, which is the percentage above printing cost that is charged to the customer. For example, an item that costs $100 to print that has a margin of 11% will cost: item: $100 -> $7 sales tax = $107 job: $100 -> $11 margin total: $100 + $7 + $11 = $118

The base margin is 11% for all jobs this problem. Some jobs have an "extra margin" of 5%. These jobs that are flagged as extra margin have an additional 5% margin (16% total) applied.

The final cost is rounded to the nearest even cent. Individual items are rounded to the nearest cent.

Write a program that calculates the total charge to a customer for a job

Samples:

Job 1: extra-margin envelopes 520.00 letterhead 1983.37 exempt

should output: envelopes: $556.40 letterhead: $1983.37 total: $2940.30

Job 2: t-shirts 294.04

output: t-shirts: $314.62 total: $346.96

Job 3: extra-margin frisbees 19385.38 exempt yo-yos 1829 exempt

output: frisbees: $19385.38 yo-yos: $1829.00 total: $24608.68

This is my code:

using System;
using System.Collections.Generic;

public namespace PrintJob
{
    class Program
    {
        static void Main(string[] args)
        {
            const decimal TaxRate = 0.07m;
            const decimal MarginRate = 0.11m;
            const decimal ExtraMarginRate = 0.16m;

            var job = new Job();
            job.ExtraMargin = true;

            job.Items = GetItemsList();

            foreach(var item in job.Items)
            {
                if (item.Exempt)
                {
                    item.Tax = 0.0m;
                }
                else
                {
                    item.Tax = item.Price * TaxRate;
                }
                if (job.ExtraMargin)
                {
                    job.Margin += item.Price * ExtraMarginRate;
                }
                else
                {
                    job.Margin += item.Price * MarginRate;
                }
                job.TotalPrice += item.Price + item.Tax;                
            }
            job.TotalPrice += job.Margin;
            job.PrintReceipt(job);

            Console.ReadLine();
        }

        private static IList<JobItem> GetItemsList()
        {
            return new List<JobItem>
            {
                new JobItem {Name = "envelopes", Price = 520.00m, Exempt = false},
                new JobItem {Name = "letterhead ", Price = 1983.37m, Exempt = true}
            };
        }
    }

    class Job
    {
        public IList<JobItem> Items { get; set; }
        public bool ExtraMargin { get; set; }
        public decimal Margin { get; set; }
        public decimal TotalPrice { get; set; }           
        public void PrintReceipt(Job job)
        {
            Console.WriteLine("Job Receipt");
            foreach(var item in job.Items)
            {                   
                Console.WriteLine("{0}:  {1} ", item.Name, Math.Round((item.Price + item.Tax),2));
            }
            Console.WriteLine("Total: {0} ", Math.Round(job.TotalPrice,2));
        }
    }

    public class JobItem
    {        
        public string Name { get; set; }
        public decimal Price { get; set; }        
        public bool Exempt { get; set; }
        public decimal Tax { get; set; }
    }

}

PS. Updated solution you can see on GitHub.

\$\endgroup\$
1
\$\begingroup\$

I've forked and refactored. Updated here.

Here's my high-level observations. I'm in a rush right now or I'd elaborate more, but let me know what you'd like clarified and I'll get back to you?

Business Data vs. Presentation

Your Job type originally had PrintReceipt as a member. I moved that out because the Job is a business object and should know as little as possible about the presentation layer as possible.

Any application doing anything generally has classes that represent the concepts being managed. These are business objects/models, and they store information. As a general rule, business objects should not have any system specific code (like printing a receipt). This allows us to use the business objects in different contexts without having to recreate them.

You can read more about clean architecture here.

Feature Envy

Because the total price is a property of the JobItem, that value should be a property of the object. Similar as with Job and the TotalPrice value. You'd want to avoid calculating them outside of the class unless there's some compelling reason to do so. This stops your code from getting tangled, and keeps related methods together.

No Interfaces

Interfaces are good. They help you unit test your code. I haven't written them but they are much easier to test with mock objects (using the Moq package, as an example) when you program to interfaces.

I didn't do it, but you should also implement interfaces for the Job and JobItem classes.

Need Separated Functions

I've split the file loading, job deserializing, and receipt printing functions into separate classes. This makes it easier to update those items later, if necessary.

As an example, I realized as I was extending this explanation that I had forgotten to change how text is output! Changing those few lines was much quicker than changing it all over the place.

Using Console to Write Text to File

Since you're going through the effort of instantiating the FileStream, I don't see any value in using the Console.WriteLine command after changing Output and Error endpoints.

I've left the UI component alone, but that should also be isolated into its own function.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.