C++

Forum

 
Shipping Calculator problems

HowieW (4)
I am having difficulty getting the total charges to appear when I run through the program. can anyone help me see the error in my ways. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
 
 #include <iostream>
#include <iomanip>

using namespace std;

int main()
{
    const int //WEIGHT_MIN = 0.00,
              WEIGHT_MAX = 50.00,
              BIG_MIN = 0.00,
              BIG_MAX = 1000.00;

    float package_weight,
    big,
    total_charges;

    cout << "\nWhat is the weight (LBS) of the package?";
    cin >> package_weight;

    if (package_weight > WEIGHT_MAX)
{
        cout << "\nWe're sorry, box is too heavy.\n"
            << endl;
}
else
{
    cout << "\nWhat is the area of the package (L x H x W in inches?) ";
    cin >> big;

    if (big < BIG_MIN)
    {
            cout << "\nWe're sorry, the area must be\n"
                << " more than 0 "
                << endl;
    }

       if (big > BIG_MAX)
    {
            cout << "\nWe're sorry, box is too large. The area must be\n"
                << "less than 1001 "
                << endl;

        if (big > BIG_MAX ||
           package_weight > WEIGHT_MAX)
    {
            cout << "\nWe're sorry, box is too large and too heavy.\n"
                << endl;
    }
    else
    {
    if (package_weight <=25)
        total_charges = (big / 5) * 100;
    else if (package_weight > 25)
            total_charges = (big / 5) * 200;

    cout << setprecision(2) << fixed
            << "Box is OK. Total charges with tax are $"
            << total_charges
            << "\nFor a area of "
            << big
            << " inches \nand a total weight of "
            << package_weight
            << " LBS. \n"

            << endl;
    }

    return 0;
}}}
zapshe (1834)
The issue is with your brackets. The "else" where you output the charges is INSIDE "if(big > BIG_MAX)" - meaning you never get to it. The charges for each package is huge though, I think you should lower your prices! Here's the functional code along with a little tweaking to the if statements:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
 
int main()
{
	const int //WEIGHT_MIN = 0.00,
		WEIGHT_MAX = 50.00,
		BIG_MIN = 0.00,
		BIG_MAX = 1000.00;

	float package_weight,
		big,
		total_charges;

	cout << "\nWhat is the weight (LBS) of the package?";
	cin >> package_weight;

	if (package_weight > WEIGHT_MAX)
	{
		cout << "\nWe're sorry, box is too heavy.\n"
			<< endl;
	}
	else
	{
		cout << "\nWhat is the area of the package (L x H x W in inches?) ";
		cin >> big;

		if (big < BIG_MIN)
		{
			cout << "\nWe're sorry, the area must be\n"
				<< " more than 0 "
				<< endl;
		}

		else if (big > BIG_MAX && package_weight > WEIGHT_MAX)
		{
			cout << "\nWe're sorry, box is too large and too heavy.\n"
				<< endl;
		}

		else if (big > BIG_MAX)
		{
			cout << "\nWe're sorry, box is too large. The area must be\n"
				<< "less than 1001 "
				<< endl;

			return 0;
		}

		else
		{
			if (package_weight <= 25)
				total_charges = (big / 5) * 100;
			else if (package_weight > 25)
				total_charges = (big / 5) * 200;

			cout << setprecision(2) << fixed
				<< "Box is OK. Total charges with tax are $"
				<< total_charges
				<< "\nFor a area of "
				<< big
				<< " inches \nand a total weight of "
				<< package_weight
				<< " LBS. \n"

				<< endl;
		}
	}
}



EDIT: You may have been able to spot the issue yourself if your code didn't look like a mess. Make sure to use proper indentations.
Last edited on
HowieW (4)
Thank you for he help! I was going to mess with the charges, I just threw something in there as a placeholder to see what it would come out to.
I know, my code is super messy. I'm sure it will get cleaner as I progress.
zapshe (1834)
I know, my code is super messy. I'm sure it will get cleaner as I progress.

It's like saying, "I'll lose weight... eventually!" You should use a good coding environment that will at least organize your brackets for you.

Good luck!
Handy Andy (5051)
Hello HowieW,

As it happens I sometimes end up with a responce that is larger than can fit in one message, so this is in two parts.

zapshe has made very good points here.To expand the compiler does not care about white space or blank lines when it compiles your program.They are just ignored.On the other hand someone reading your code, including your - self, does care about how it looks.So the proper indenting along with a judicious use of blank lines makes the code easier to read and understand.Since you are writing the code for someone to read, especially when you want help, make it as easy to understand as possible.

And now is the time to learn a better way of coding.Not later when bad habits are hard to change.

Here is your program made to work.Not the best way to go about it, but I did manage to get it to work.Be sure to read the comments I put in the program.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
 
#include <iostream>
#include <iomanip>
#include <limits>

//using namespace std;  // <--- Best not to use.
// A recent post that is worth reading. http://www.cplusplus.com/forum/beginner/258335/

int main()
{
	const double
		WEIGHT_MIN = 1.0,
		WEIGHT_MAX = 50.0,
		BIG_MIN = 1.0,
		BIG_MAX = 1000.0,
		MIN_CHARGE{ 1.5 },
		MAX_CHARGE{ 2.5 },
		DIVISOR{ 5.0 },
		CHARGE_WEIGHT{ 25.0 };

	const double TAX_RATE{ 0.05 };

	double package_weight{},
		big{},
		total_charges{},
		length{},
		width{},
		depth{};

	std::cout << std::fixed << std::showpoint << std::setprecision(2); // <--- Only needs done once.

	std::cout << "\n What is the weight (LBS) of the package? ";
	std::cin >> package_weight;

	while (!std::cin || (package_weight < WEIGHT_MIN || package_weight > WEIGHT_MAX))
	{
		if (!std::cin)
		{
			std::cout << "\n    Invalit entry. Please enter a number greater than 0 and " << WEIGHT_MAX << " or less.\n";

			std::cin.clear(); // <--- Resets state bits.
			std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>. Clears the input buffer.
		}
		else if (package_weight < WEIGHT_MIN || package_weight > WEIGHT_MAX)
		{
			std::cout << "\n    Invalid weight! Weight should be greater than 0 and " << WEIGHT_MAX << " or less.\n";
		}

		std::cout << "\n What is the weight (LBS) of the package? ";
		std::cin >> package_weight;
	}

	if (package_weight > WEIGHT_MAX)
	{
		std::cout << "\n    We're sorry, box is too heavy.\n"
			<< std::endl;
	}
	else // <--- Never executed if the previous if statement is true.
	{
		std::cout << "\n What is the area of the package (L x H x D in inches)?";
		std::cout << "\n  Enter Length: ";
		std::cin >> length;
		std::cout << "  Enter Width: ";
		std::cin >> width;
		std::cout << "  Enter Depth: ";
		std::cin >> depth;

		big = length * width * depth;

		if (big < BIG_MIN)
		{
			std::cout << "\n    We're sorry, the area must be"
				<< " more than 0 "
				<< std::endl;
		}
		else if (big > BIG_MAX)
		{
			std::cout << "\n    We're sorry, box is too large. The area must be"
				<< "less than 1001 \n"
				<< "    You have entered a size of " << big << '\n'
				<< std::endl;

			//if (big > BIG_MAX && package_weight > WEIGHT_MAX)
			//{
			//	std::cout << "\n We're sorry, box is too large and too heavy.\n" // <--- Never reached.
			//		<< std::endl;
			//}
		}
		else
		{
			if (package_weight <= CHARGE_WEIGHT)
				total_charges = (big / DIVISOR) * MIN_CHARGE;
			else if (package_weight > CHARGE_WEIGHT)
				total_charges = (big / DIVISOR) * MAX_CHARGE;

			std::cout << "\n\n Box is OK.\n Total charges with tax are $"
				<< total_charges
				<< "\n For a area of "
				<< big
				<< " inches \n and a total weight of "
				<< package_weight
				<< " LBS. \n"
				<< std::endl;
		}
	}

	// The next line may not be needid. If you have to press enter to see the prompt it is not needed.
	std::cin.ignore(std::numeric_limits<std::streamsize>::max(), '\n');  // <--- Requires header file <limits>.
	std::cout << "\n\n Press Enter to continue :";
	std::cin.get();

	return 0;
}

At the top I added the header file "<limits>" for the
1
2
3
4
 
std::cin.ignore(...)[/ code] I used.

Inside "main":
I changed and added to the constant variables.In your original code[code]const int WEIGHT_MIN = 0.00,
defining the variable as an "int" and then initializing it with a floating point number may work, but is not the proper way.The "0.0", as it should be written is considered a "double" and when stored the ".0" is dropped an only the whole number is stored.This is not a big problem, beyond the compiler warning, until there is something to the right of the decimal point and then you would have data loss.

For the way the program is written these constant variables work better as "double"s not "int"s.And these days "double" is the preferred floating point type and even then it can cause problems with calculations as it does not store some decimal numbers correctly, e.g., "0.03" may be stored as "0.029999999999999999". This is not a problem when printing only two decimal places as it rounds up properly, but can affect a calculation.Just so you know.

The other thing is that with "0.00" or "50.00" only one zero is needed.the extra zero does not help or hinder.

Theconst double TAX_RATE{ 0.05 }; was a thought I had, but have not done anything with yet, but it does show storing a smaller number.

The next group of variables are initialized to zero with the empty{}. This may or may not be needed, but at least I know they do not contain a garbage value.

Line 29 as the comment says only needs done once in the program and will affect all "cout" statements the print a floating point number until the program ends or something is changed.I believe you understand the "std::fixed" part.The "std::showpoint" means that if the number is "50.0" show the decimal point and the zero.Otherwise it would drop the ".0" and only show the whole number.This is useful when lining up the decimal points of the output.

Using formatted input as withstd::cin >> package_weight; the "cin" is expecting a number to be entered and anything that is not a number will cause "cin" to fail and become unusable the rest of the program unless corrected.

The while loop may be ahead of where you are at, but it is a good time to learn about it.

The while condition is first checking if the "cin" stream is in a good state, i.e., if the good bit is 1, if this has changed to(0)zero then you enter the while loop to deal with it.The second part is checking if "package_weight" is in the proper range.

The "if/else if" allows it to print one error message and not both.Using the "if/else if" combination allows for only need one prompt and "cin" to get a new number.When you have a proper number in the proper range the while loop is finished.

The while loop allows you to correct any problem instead of ending the program and having to start over.Once you get use to it you will find it better than what you start with in your program.

End part one.

Edit: Tags did not come out right.
Last edited on
Handy Andy (5051)
Part two.

I was doing really good until I reached line 52. First I realize that the preceding while loop negates the need for the if and else here as the package weight should be known before the program is run and if over 50Lbs it would be rejected before the program is needed, so checking for the package weight being over 50 becomes irrelevant.

Then I had more questions than answers.I have no idea what you expect from the program.Or what your original instructions are.My years of learning are making me think of what is wrong with the program and what should be changes.

If this is for school that is fine, but post the instructions for the program so everyone has a better idea of what is needed.Later in the program you print "Total charges with tax are". This may appear OK, but from an accounting point of view it becomes a challenge to figure what part of the total is tax.

Now when you get to entering the "length" "width" and "depth" you are expecting the person running the program to figure the total area of the package.This is not a good idea, even with instructions, some people will have no idea what to do or how to do it.

Following each "cin >>" it would be a good idea to follow this with a new version of the above while loop.At least to check for "!std::cin". Checking for the minimum size of each is making sure it is at least(1). The upper size could be anything unless you want to limit the size.The following calculation for "big" will use the next if statement to check the total area.

At line 82 the if statement is not needed.First off it should be an "&&" not "||" because both sides need to be true for the error message to be useful.Tor the lhs "big > BIG_MAX" has already been taken care by the above code and the rhs "package_weight > WEIGHT_MAX" will never be true because it was dealt with earlier.

By removing the if statement this puts the else part with the "else if" on line 75 allowing the "cout" statement to work when it is needed.

The lines of code :
1
2
3
4
 
if (package_weight <= CHARGE_WEIGHT)
    total_charges = (big / DIVISOR) * MIN_CHARGE;
else if (package_weight > CHARGE_WEIGHT)
    total_charges = (big / DIVISOR) * MAX_CHARGE;

Are a better way of writing this than what you originally start with.The 100 ans 200 you refer to as place holders may be easy to change in a small program, but in a larger program say with a 1000 lines of code it would be easy to miss a change that may not show up immediately.

Your original code :
1
2
3
4
 
if (package_weight <= 25)
    total_charges = (big / 5) * 100;
else if (package_weight > 25)
    total_charges = (big / 5) * 200;

The numbers that you have are considered "magic" numbers and should be avoided.Using the constant variables are a better place holder and easier to change.Plus if they should be used more than once the constant variables will make the change every where that is needed.

When you divided by "5" it would be better written as "5.0".The "5" is considered an "int" and even though the compiler would promote this to a "double" for the calculation it is better to write "5.0".Just so you know.

Having no idea what the "5" is for or why "5" I called it "DIVISOR", but you are free to use any name that fits better.

The last lines before the "return" are not necessary.It is something I use to keep the window open with the way I have my IDE set up.Good lines for the future if you need a pause in a program.

End part two.

Hope that helps,

Andy

Edit: Tags did not come out right. Sorry about that.
Last edited on
Topic archived. No new replies allowed.