file.get() returning random numbers after a loop C++

Go To StackoverFlow.com

0

I am working on a program that reads a series of ints from a text file into a 2D array.
The file contains 40 lines of 81 numbers that have no spaces between them.

The problem is that when I cout the array after the loop has finished, it is outputting 2 random numbers in array[0][0] and array[0][1] before the expected output. I think it has to do with the newline/carriage return characters. The first iteration of the loop runs perfectly. Here is the code:

#include <cstring>
#include <cstdlib>
#include <iostream>
#include <fstream>

using namespace std;

int main()
{
  int array[9][9];

  //Open file:

  fstream ifile;

  ifile.open("numbers.txt", ios::in);
  if (ifile.fail())
    {
      cout << "Could not open numbers.txt" << endl;
      return -1;
    }

   while (!ifile.eof())
    {
      for(int i=0; i<9; i++)
      {   
    for(int j=0; j<9; j++)
    {
       int n = ifile.get();
              if(isdigit(n)) 
            {
                  array[i][j] = n - '0';
        }

          cout<<"array ["<<i<<"]["<<j<<"] is "<<array[i][j]<<endl;
      } 
      } cout<<"This is a test"<<endl;
    }

  return 0;
}
2012-04-05 16:22
by adohertyd
How does your input file look like - Kornel Kisielewicz 2012-04-05 16:29
1) You don't test to see if file.get() succeeds. 2) You increment j even if n is not a digit, and 3) Please provide a short, complete program that demonstrates the error you are having. See http://sscce.org/ - Robᵩ 2012-04-05 16:31
@KornelKisielewicz it's a file with 40 lines of 81 numbers that have no spaces between the - adohertyd 2012-04-05 16:32
@Robᵩ That's only a snippet of code. The program is quite long so I only included the bit that was causing problems - adohertyd 2012-04-05 16:34
And you tested this snippet by itself - Beta 2012-04-05 16:37
@adohertyd - I'm not suggesting that you give us "the program". As you can see at http://sscce.org, it would be helpful for you to give us some program that is complete, short, and demonstrates the problem. You can create such a program by starting with your program, removing bits and continuously testing the result. Once you have the shortest possible program that still fails, post that - Robᵩ 2012-04-05 16:38
possible duplicate of Why is iostream::eof inside a loop condition considered wrong?Ben Voigt 2012-04-05 16:49
@BenVoigt Not a duplicate post completely different proble - adohertyd 2012-04-05 17:05
@adohertyd If it's a file of 40 lines of 81 digits, then you're going to have a lot of difficulty reading it into an array of 9x9 - James Kanze 2012-04-05 17:05
@JamesKanze I'm reading each 81 digit line into a 9x9 array, processing that particular line(the process is not included in my code) and then reading the next line in etc. to end of fil - adohertyd 2012-04-05 17:08
@adohertyd: The fact that you think this is a completely different problem explains why you haven't gotten it working yet - Ben Voigt 2012-04-05 17:13
@BenVoigt My problem occurs after the first loop takes place. eof isn't causing the problem I'm experiencing. If I change the while loop to for(k=0; k<5; k++) the problem remains the same - adohertyd 2012-04-05 17:19
@adohertyd In which case, the best solution is probably to use std::getline to read the line into a string, and then process the string for the individual characters - James Kanze 2012-04-10 08:32


1

The random numbers appear because you increment j regardless of whether you write to grid[i][j].

Try replacing your inner loop with:

for(int j=0; j<9; )
{
  int n = file.get();
  if(!file) break;
  if(isdigit(n))
  {
    array[i][j] = n - '0';
    cout<<"array ["<<i<<"]["<<j<<"] is "<<array[i][j]<<endl;
    j++;
  }
  cout<<"grid ["<<i<<"]["<<j<<"] is "<<grid[i][j]<<endl;
} 
2012-04-05 16:42
by Robᵩ
For what it's worth I've included a working program Ro - adohertyd 2012-04-05 17:03
It was worth a great deal. I've edited my answer. It provides a correct solution now - Robᵩ 2012-04-05 17:58


2

I don't understand the purpose of the outer loop at all. First, file will never be equal to eof(), or... What is eof(), any way? Second, if you've actually written while ( !file.eof() ), this could explain the fact that some of the elements get overwritten. There will likely be some trailing characters (a new line, at least) after the last digit, so you'll reenter the loop again.

And you're incrementing the indexes even when the character you read is not a digit. If the data is 9 lines of 9 digits, you'll end up with 9 cells in grid which haven't been initialized, and 9 characters that haven't been read from the file once you've finished the inner two iterations. So you'll enter the outer loop again, reading these characters. Some of which will be digits, so you'll end up overwriting cells in grid that you've already written—this is probably the effect you're observing. Also that once you reach the end of the file, file.get() will start returning EOF—typically -1. That's doubtlessly why your tests for '\n' and '\r' didn't work.

And these are just the problems with a correctly formatted file. For a correctly formatted file, just using file >> n, with char n; would almost work; operator>> skips whitespace. But you'd still enter the outermost loop a second time, since file.eof() won't be reliable until an input has failed. You say "I have to use this", but your code can't be made to work unless you change it.

Personally, I favor robust solutions, with lots of error checking. I'd use std::getline(), and I'd verify that each line contained exactly 9 digits. Something like:

std::string line;
int i = 0;
while ( i < 9 && std::getline( file, line ) ) {
    if ( line.size() != 9 ) {
        throw FormatError( "wrong line length" );
    }
    for ( int j = 0; j != 9; ++ j ) {
        if ( ! isdigit( static_cast<unsigned char>( line[j] ) ) ) {
            throw FormatError( "illegal character" );
        }
        grid[i][j] = line[i] - '0';
    }
}
if ( i != 9 || std::getline( file, line ) ) {
    throw FormatError( "wrong line count" );
}

It wouldn't be too hard to use file.get(), and read one character at at time, but you'd still want to check for EOF after each read:

for ( int i = 0; i != 9; ++ i ) {
    for ( int j = 0; j != 9; ++ j ) {
        int ch = file.get();
        if ( ch == EOF ) {
            throw FormatError( j == 0 
                                ? "line too short" 
                                : "too few lines" );
        }
        if ( !isdigit( ch ) ) {
            throw FormatError( "illegal character" );
        }
        grid[i][j] = ch - '0';
    }
    int ch = file.get();
    if ( ch != '\n' ) {
        throw FormatError( ch == EOF ? "too few lines" : "line too long" );
    }
}
if ( file.get() != EOF ) {
    throw FormatError( "too many lines" );
}
2012-04-05 17:02
by James Kanze


1

eof isn't set when you reach the end of the file, it's set after a read fails. And the data from that last read is of course invalid, because it failed. Your current code uses the invalid data...


Beyond that, file != eof() is all kinds of wrong. It shouldn't even compile, because there is no ::eof() function, and iostream::eof() needs an object. file != EOF might compile, but then file will be converted to bool and promoted to int (either 0 or 1), it will never be equal to EOF (-1). What you meant was !file.eof(), but that's also wrong for the reason given above.

2012-04-05 16:45
by Ben Voigt
Your statements are true of course. But that bug doesn't explain his complaint. Namely "2 random numbers in array[0][0] and array[0][1]" - Robᵩ 2012-04-05 16:46
@Rob: Sure it does, he calls file.get() many times before he checks eof() - Ben Voigt 2012-04-05 16:47
@Rob It does, because by advancing his indexes faster than he consumes digits, he ends up taking the outer loop twice, overwriting some of the earlier digits - James Kanze 2012-04-05 17:04
Ads