HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Calculate total cost of items, based on bulk discount

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
totalbulkitemscalculatebaseddiscountcost

Problem

I've written a short program for one of my first exercise in C++.... I would like to get better suggestions to solve it, so hope you can help me :) And I hope this is the right session to ask...:)


Exercise:


Input: Pieces and price per piece. If pieces > 10, its 5% discount. If pieces>50, 10% discount


Output: total price

Thank you a lot in advance

#include 
#include 
using namespace std;
int main() {
int stk, ep, gesamt;
cout > stk;
cout >ep;
if (stk>=10)
{
    if (stk>=50)
    //10% discount
    {
    gesamt=stk*ep;
    gesamt=gesamt - (gesamt*10)/100;
    goto stop;
    }
     else   //5% discount
    gesamt=stk*ep;
    gesamt=gesamt - (gesamt*5)/100;
    goto stop;
}

else gesamt=stk*ep;

stop:
    cout << gesamt << endl;

return 0;
}

Solution

Avoid using namespace std

This can cause name collisions because it adds every name in the std namespace to the global namespace. For a small program like this one it's unlikely that you'll run into any problems (then again, maybe not) but it's best to get into the habit of using the std:: prefix on names in the std namespace.

Alternatively, you can introduce using declarations like using std::cout; to add specific names to the global namespace.

Avoid duplicated code like gesamt=stk*ep;

That statement appears three times in your code. If that logic ever changes you'll need to make sure you change it everywhere, and you might miss one or more instances. Put that duplicated code in a function if necessary, or just write it once if possible.

Indent for readability

None of the code in main() is indented, for example. It's easier to read your program if it's indented like this:

int main() {
  int stk, ep, gesamt;
  //...
}


Use more whitespace for readability

E.g. use gesamt = stk ep; instead of gesamt=stkep;

Use descriptive variable names

The variable names seem to be in a different language (German?) but stk and ep in particular look like they are abbreviations or acronyms. I would prefer pieces for stk and unit_price for ep.

Avoid std::endl in favor of \n

std::endl flushes the stream, which can cause a loss in performance. In some cases you can also reduce calls to operator 10 but you use >=. Your code applies a discount if pieces = 10 but the problem statement implies that the discount should not be applied in that case (only starting with pieces = 11).

Use the appropriate type for variables

In this case, you probably need to use
float or double for ep and gesamt. The price isn't necessarily an integer and neither is the total price if the discount is applied.

Check your inputs for error conditions

For example, neither the number of pieces nor the unit price should be negative. Check for these error conditions and handle them appropriately (output an error message, throw an exception, ask for a new input, etc.).

Avoid unnecessary
goto statements

goto is generally not needed, and that is the case in your program. Your program will reach the stop statement after entering the various if/else clauses.

Result

Here's the end result of your program with the above suggestions (I've also made the code more concise in some cases, e.g. by the use of the
*=` operator to multiply a number by itself):

#include 

int main() {
    int pieces;
    double unit_price;
    double gesamt;

    std::cout > pieces;

    if (pieces > unit_price;

    if (unit_price  10)
    {
        if (pieces > 50)
        //10% discount
        {
        gesamt *= 0.9;
        }
        else   //5% discount
        {
        gesamt *= 0.95;
        }
    }

    std::cout << gesamt << '\n';

    return 0;
}

Code Snippets

int main() {
  int stk, ep, gesamt;
  //...
}
#include <iostream>

int main() {
    int pieces;
    double unit_price;
    double gesamt;

    std::cout << "Pieces?\n";
    std::cin >> pieces;

    if (pieces < 0)
    {
        std::cout << "Cannot have negative pieces!\n";
        return -1;
    }

    std::cout << "Price per Piece?\n";
    std::cin >> unit_price;

    if (unit_price < 0)
    {
        std::cout << "Cannot have negative unit price!\n";
        return -1;
    }

    // Calculate total price
    gesamt = pieces * unit_price;

    // Apply discount, if applicable
    if (pieces > 10)
    {
        if (pieces > 50)
        //10% discount
        {
        gesamt *= 0.9;
        }
        else   //5% discount
        {
        gesamt *= 0.95;
        }
    }

    std::cout << gesamt << '\n';

    return 0;
}

Context

StackExchange Code Review Q#143120, answer score: 5

Revisions (0)

No revisions yet.