Simple C++ input files and if statements

Question

I wrote the code and it works except the total is wrong. It is supposed to multiply the distanceRate by the rate and add each cost to make the total, but it's not doing that. Any help would be appreciated.

#include <iostream>
#include <string>
#include <iomanip>
#include <fstream>

using namespace std;

int main()
{
    //Declare Variables
    ifstream inFile;

    double packageWeight;
    double distance;
    double totalCharge = 0;
    double rate;
    double distanceRate;

    int customerNumber;
    double shippingCharge;
    int packageCount = 0;


    inFile.open("shipping.txt");
    if(inFile)
    {
        cout << "Customer   Package   Shipping" << endl;
        cout << "Number     Weight    Distance" << endl;

        while(!inFile.eof())
        {
            inFile >> customerNumber;
            inFile >> packageWeight;
            inFile >> distance;

            if(0 < packageWeight <= 2)
                rate = 1.10;
            else if(2 < packageWeight <=6)
                rate = 2.20;
            else if(6 < packageWeight <= 10)
                rate = 3.70;
            else if(10 < packageWeight <=20)
                rate = 4.80;
            else
                cout << "Invalid package weight" << endl;

            if( 0 < distance <= 500)
                distanceRate = 1;
            else if( 500 < distance <= 1000)
                distanceRate = 2;
            else if(1000 < distance <= 1500)
                distanceRate = 3;
            else if(1500 < distance <= 2000)
                distanceRate = 4;
            else
                cout << "Invalid distance" << endl;

            packageCount += customerNumber;
            shippingCharge = rate * distanceRate;
            totalCharge += shippingCharge;

            cout << fixed << setprecision(2) << showpoint;
            cout << setw(2) << customerNumber
            << right << setw(14) << packageWeight
            << setw(13) << distance
            << endl;

        } //End of while loop

        cout << "\nPackage shipped : " << packageCount << endl;
        cout << "Total Charge : $" << totalCharge << endl;
        inFile.close();
    }
    else
    {
        cout << "Could not open file" << endl;
    }
    system("pause");
    return 0;
}

Answer 1

Some issues that I see in the snippet you gave me are as follows:

1. As pointed out by billz in a comment, your if statements are invalid. The statement if( 0 < distance <= 500) is not doing what you expect, it evaluates from left to right, so you have 0 < distance (lets say that evaluates to true) so then you have true <= 1000 which isn't going to give the results that you think it will. This actually needs to be broken apart into two separate comparisons like distance > 0 && distance < 500.

2. As I noted in my comment, you're adding the customer number to the package count, this will most likely always give a wrong value for package count. If your customer numbers are 1, 2, 3, 4 then you claim the package count is 10 when it's actually only 4 (forgive me if I misunderstood the purpose of this field).

3. You have no default value for distanceRate but you still use it in an operation (possibly uninitialized) which will give unexpected results (as you are seeing). In your else, you should actually give it a dummy value that way you guarantee that it will always be set. You also do reset it, so if it gets set to 4, and then next distance fails the tests and enters the else, you have another calculation on the variable as 4 instead of it's default value. You should initialize any variable that you plan to use unless you have explicit reason not to give it a value at initialization, and anytime you use a variable in a loop you should reset it's value at the start of the loop.

Additional Note (EDIT)

---

I wouldn't recommend using system("pause"); as it does a lot more behind the scenes than you would want in a simple pause, a better approach I've seen used is:

#include <iostream>
#include <conio.h>
using namespace std;

int main() {
    cout << "Press any key to continue!";
    _getch();
    cout << "Finished";

    return 0;
}

EDIT 2

---

If statments can contain a single line or a code block to execute.

Single line:

if (someValueIsTrue)
    executeThisFunction();

Code block:

if (someValueIsTrue) {
    executeThisFunction();
    alsoThisFunction();
}

Anytime you need to execute more than one statement in an if/else/while/for/do...while/etc... you'll need a code block. I imagine (based on your explanation) that you did:

if (blah)
    // ....
else
    distanceRate = 0;
    cout << "Invalid Distance";

And the compiler only sees that you have the distanceRate = 0 nested in the loop, the cout statement is actually not part of the else but part of the previous block of code. You need to use a code block here.

Answer 2

!inFile.eof()  // incorrect
 inFile.good() // correct

read on eof() it doesn't do what you might think it does.

if( 0 < distance <= 500) // all the if statements are incorrect
if(distance>0 && distance<=500) // correct

The way you wrote the if condition, it does not do what you think it does.