While loop with end of file function repeats last number in output file twice

Question

Hi I know this might sound like a repeat question but I have spent many days on this problem and cannot find out a solution. In my code I have a while(true) loop and at the end of the file I have an if statement with the end of file function and break but in the output it repeats the last number twice.

#include <fstream>
#include <iostream>
#include <iomanip>
#include <cmath>
#include <vector>
#include <algorithm>
#include <string>
using namespace std;
int main ()
{

int line=0;

ifstream    infile; //INPUT input file stream
ofstream    outfile; //OUTPUT output file stream

infile.open("inputPartb.txt");
if (! infile)
{
    cout <<"Problem opening input file inputPartb.txt"<<endl;
    return 1; //not successful
}

outfile.open("resultsPartb.txt");
    if (! outfile)
    {
        cout <<"Problem opening output file resultsPartb.txt"<<endl;
        return 1; //not successful
    }


while (true)
{
    double num1;
    double num2;

    int intnum1;
    int intnum2;

    char mathOperator;

    double addition;
    double subtraction;
    double multiplication;
    int division; //division is using remainders so float isnt needed
    double power;

    double remainder;

    line = line +1;


    //reading numbers from the file
    infile >> num1 >> num2 >> mathOperator;

    intnum1 = num1;
    intnum2 = num2;


    //if statement for addition
    if (mathOperator == '+')
    {
        //one for both num1 and num2 being integers
        if ((num1-intnum1==0)&&(num2-intnum2==0))
        {
            addition = num1+num2;
            outfile << fixed<< setprecision(0)<< num1 << " + " << num2 << " = " <<addition <<endl;
        }
        //one for either num1 or num2 being a float
        else
        {
            addition = num1+num2;
            outfile << fixed << setprecision(4)<< num1 << " + " << num2 << " = " <<addition <<endl;
        }

    }


    //if statement for subtraction
    else if (mathOperator == '-')
    {
        //one for both num1 and num2 being integers
        if ((num1-intnum1==0)&&(num2-intnum2==0))
        {
            subtraction = num1-num2;
            outfile << fixed<< setprecision(0)<< num1 << " - " << num2 << " = " <<subtraction <<endl;
        }
        //one for either num1 or num2 being a float
        else
        {
            subtraction = num1-num2;
            outfile << fixed<< setprecision(4)<< num1 << " - " << num2 << " = " << subtraction <<endl;
        }

    }
    //if statement for multiplication
    else if (mathOperator == '*')
    {
        //one for both num1 and num2 being integers
        if ((num1-intnum1==0)&&(num2-intnum2==0))
        {
            multiplication = num1*num2;
            outfile << fixed<< setprecision(0)<< num1 << " * " << num2 << " = " <<multiplication <<endl;
        }
        //one for either num1 or num2 being a float
        else
        {
            multiplication = num1*num2;
            outfile << fixed<< setprecision(4)<< num1 << " * " << num2 << " = " << multiplication <<endl;
        }

        continue;
    }
    //if statement for division
    //one for both num1 and num2 being integers
    else if (mathOperator == '/')
    {
        if ((num1==intnum1)&&(num2==intnum2))
        {
            if (num2 == 0)
            {
                outfile << num1 << " / " << num2 << " = ERROR" <<endl;
                cout << "ERROR encountered on line "<<line<<endl;

            }
            else if ((num2==0)&&(num1==0))
            {
                division = static_cast<int>(num1)/static_cast<int>(num2);
                remainder = (static_cast<int>(num1) % static_cast<int>(num2));
                outfile <<setprecision(0)<<fixed<< num1 << " / " << num2 << " = " << division<<"R"<<remainder<<endl;
            }
            else
            {

                division = static_cast<int>(num1)/static_cast<int>(num2);
                remainder = (static_cast<int>(num1) % static_cast<int>(num2));
                outfile <<setprecision(0)<<fixed<< num1 << " / " << num2 << " = " << division<<"R"<<remainder<<endl;
            }
        }
        //one for either num1 or num2 being a float
        else
        {
            if (num2 == 0)
            {
                outfile << num1 << " / " << num2 << " = ERROR" <<endl;
                cout << "ERROR encountered on line "<<line<<endl;

            }
            else if ((num2==0)&&(num1==0))
            {
                division = static_cast<int>(num1)/static_cast<int>(num2);
                remainder = (static_cast<int>(num1) % static_cast<int>(num2));
                outfile <<setprecision(4)<<fixed<< num1 << " / " << num2 << " = " << division<<"R"<<remainder<<endl;
            }
            else
            {

                division = static_cast<int>(num1)/static_cast<int>(num2);
                remainder = (static_cast<int>(num1) % static_cast<int>(num2));
                outfile <<setprecision(4)<<fixed<< num1 << " / " << num2 << " = " << division<<"R"<<remainder<<endl;
            }
        }

    }
    //if statement for power
    else if (mathOperator == '^')
    {
        if ((num1-intnum1==0)&&(num2-intnum2==0))
        {
            if ((num1 == 0)&& (num2 == 0))
            {
                outfile << num1 << " ^ " << num2 << " = " << "ERROR" <<endl;
                cout << "ERROR encountered on line "<<line<<endl;
            }
            else
            {
                power = pow(num1 , num2);
                outfile <<setprecision(0)<<fixed<< num1 << " ^ " << num2 << " = " << power <<endl;
            }
        }
        else
        {
            if ((num1 == 0)&& (num2 == 0))
            {
                outfile << num1 << " ^ " << num2 << " = " << "ERROR" <<endl;
                cout << "ERROR encountered on line "<<line<<endl;
            }
            else
            {
                power = pow(num1 , num2);
                outfile <<setprecision(4)<<fixed<< num1 << " ^ " << num2 << " = " << power <<endl;
            }
        }

    }
    //else statement for something other than the listed choices
    else
    {
        if ((num1-intnum1==0)&&(num2-intnum2==0))
        {
            outfile <<setprecision(0)<<fixed<<num1 << " " << mathOperator << " "<< num2 << " = ILLEGAL"<<endl;
            cout <<"ILLEGAL operator encountered on line "<<line<<endl;
        }
        else
        {
            outfile <<setprecision(4)<<fixed<< num1 << " " << mathOperator << " "<< num2 << " = ILLEGAL"<<endl;
            cout <<"ILLEGAL operator encountered on line "<<line<<endl;
        }
    }



if( infile.eof() ) break;

}
outfile.close();
infile.close();


return 0;

}

Any help would be greatly appreciated.

Do not loop on eof. You need to check if last read operation was succesfull _before using values you got from it_ and break if it was not. — Revolver_Ocelot, Jun 29 '17 at 06:14

Aconcagua · Answer 1 · 2017-06-29T07:18:32.577

The eof-check is problematic, for two reasons:

If any read operation fails, the fail bit of the stream gets set and any subsequent read will fail, too. In consequence, the stream does not advance any more and eof will never be reached, so you are caught in an endless loop, repeately printing out the last successful operation, if any, otherwise undefined behaviour, as you then evaluate at least one uninitialised value (operator>> does not touch the value on failure!).
You read in the operator as a character, so operator>> will simply consume this one character. Probably, though, your file is terminated by a newline character, which is not yet consumed, so eof bit will not be set. Next read, however, will then fail (both fail and eof bits will be set, so your loop will exit), but as you do this check after evaluating your values, you will repeat the last successful operation again, analogously as described in 1. However, this time your loop does exit because of eof-bit being set...

So rather check this way:

while (true)
{
    infile >> num1 >> num2 >> mathOperator;
    if(!infile) // possible as ifstream offers a bool cast operator...
        break;

    // evaluate values now
}

You can use eof(), if you want more fine grained handling, but then check fail(), too:

while (true)
{
    infile >> num1 >> num2 >> mathOperator;
    if(infile.eof()) 
        break;
    if(infile.fail())
    {
        // report error
        break;
    }
}

Some side notes:

Comparing floating point values with == checks for absolute equality. But due to rounding that can occur (we have limited number of bits, so precision is limited, too), this can fail badly. Have a look at here, especially follow the links provided.

Additionally, you have lots of duplicate code, you should really avoid.

First thing you can do:

infile >> num1 >> num2 >> mathOperator;
if(!infile)
    break;
int precision = num1 - intnum1 == 0 && num2 - intnum2 == 0 ? 0 : 4;

Now you can use precision while outputting:

outfile << fixed << setprecision(precision);

And you can drop most of the if/else entirely right away. Even if you did not, you should prefer this:

addition = num1 + num2; // common code, keep outside!
if (/*...*/)
{
    outfile << fixed << setprecision(0);
}
else
{
    outfile << fixed << setprecision(4);
}

For addition, subtraction and multiplication, you could provide a common handling:

double result;
if(op == `+`)
     result = num1 + num2;
else if(op == `-`)
     result = num1 - num2;
else if(op == `*`)
    result = num1 * num2;
else
    goto OTHERS;

outfile << fixed << setprecision(precision) << num1 << op << num2 << '=' << result << endl;
    continue; 

OTHERS:
    // do the rest...

OK, before any one complains: you might have read about goto being evil. Well, no, that is not true. It is not evil per se, but you can do evil things with - like with an axe, you can use it to cut wood, but you can use it, too, to kill people... Compare the following (left some lines out, so no valid C++!):

if(op == `+`)
else if(op == `-`)
else if(op == `*`)
// else
// ^ we leave it away entirely!

if(op == `+` || op == `-` || op == `*`) // we repeate the checks!
{
    outfile << fixed << setprecision(precision) /* ... */;
}
else
{
    // the LARGE block here has an additional level of indentation
    // alternatively, we could have used continue in the if...
}

or this:

if(op == `+`)
else if(op == `-`)
else if(op == `*`)
else
{
     // very LARGE block of code

     continue; // this one, you miss easily!
}

// and the common code to the initial if's is located FAR away from the them...

Honestly, doesn't the goto make the code clearer?

That you so much this reply shortened my program by at least 50 lines and I fixed the problem!! — Nikhil Karnik, Jun 29 '17 at 08:21

While loop with end of file function repeats last number in output file twice

1 Answers1