Read matrix from file -> eof() or not?

[gedamial]

I'm reading a Matrix from a file.

The file looks like this

TMMMTTT
MMMMTMM
TTMTTMT
TMMTMMM
TTTTTTM
TTTTTTM

And my code looks like this

while (!f.eof())
   for (int r = 0; r < 6; r++)
      for (int c = 0; c < 7; c++)
         f >> pizzaMatrix[r][c];

Now I have a doubt about the WHILE() CONDITION


Should it be

while (!f.eof())

or

while (f)

They both give me the same result... but which one is better? why?

When to use one and when to use the other?

PS: I already know EOF() is a flag that becomes TRUE when trying to read past the end of the file.

[Cubbi]

"while(!f.eof())" is always either an error or (when the error is worked around) dumb code. Same goes for "while(f)" in this case.

What you are supposed to do is check if an I/O operation failed *after* that operation is attempted, but *before* using the result of the operation.

in your case, regardless of the choice between "f.eof()" and "f" the for-loops will perform 6x7 attempts to read from the file (since you never check if any of those input operations failed). If your file contains just that matrix, after reading the last letter, the while loop runs *again* (the last operation didn't fail, eof is false, (bool)f is true), and the next attempt to read from the file will cross eof, raising the flag and (as of C++11) writing zero to pizzaMatrix[0][0]. Your for-loop will then perform the rest of its second 6x7 attempts to read from a failed stream, filling (as of c++11) the rest of pizzaMatrix with zeroes. Then the failure is finally checked by that while condition and the loops end.

A normal C++ reading loop is while(/*input operation*/) { /* use the result */ }, such as while(f >> n) { v.push_back(n); }. If I were you, I'd make that matrix a class with operator>>, to write PizzaMatrix pm; if(f >> pm) { eat_the_pizza(pm); /* or whatever*/ }

Edit: oops, you're presumably using operator>> for chars, which doesn't zero out its target on failure even after C++11 (only numeric input does that). so your array is probably safe from modification on the second iteration of the while loop. But it's still dumb code.

[gedamial]

the next attempt to read from the file will cross eof, raising the flag and (as of C++11) writing zero to pizzaMatrix[0][0].

Writing zero to pizzaMatrix[0][0]? What do you mean?
My matrix, at the end of the while-loop, looks exactly like the one described in the file.

There's no 0

Your for-loop will then perform the rest of its second 6x7 attempts to read from a failed stream, filling (as of c++11) the rest of pizzaMatrix with zeroes

Again, my matrix has no zeroes.

A normal C++ reading loop is while(/*input operation*/) { /* use the result */ }

char c;

for (int r = 0; r < rows; r++)
	for (int c = 0; c < columns; c++)
		while (f >> c)
		pizzaMatrix[r][c] = c;

but the output is

=======
=======
=======
=======
=======
=======

... i guess i still need to check eof()

[ne555]

¿why do you have the while in the first place?
You want to read one matrix, so read it one time and do something else already.


About your second code. It makes no sense, try to draw its diagram flow to understand what you are doing. Also `c' has too many meanings, it identifies the cell that you are reading and also is supposed to contain its value.

[Cubbi]

There's no 0

Right, you're reading chars rather than ints, so failed input doesn't wipe out the targets (I edited my original post when I realized that). It's still doing one extra spin of the while loop since "while eof" is wrong.

[gedamial]

But it's still dumb code.

How should I make it then?

I tried the

while(/*input operation*/) { /* use the result */ }

method (the code above) but it doesn't work

I also tried this one (working)

for (int r = 0; r < rows; r++)
	for (int c = 0; c < columns; c++)
		f >> pizzaMatrix[r][c];

which has no check

[Cubbi]

gedamial wrote:

How should I make it then?

It's hard to tell without knowing the whole story. As I said, I would have written it this way: while(f >> pm) { eat(pm); }, assuming "eat()" is what you planned to do with those pizza matrices. Perhaps you're not planning to read multiple pizzaMatrices at all and ne555's suggestion is the simplest: drop the "while" loop.

I was really answering the initial question "which one is better? why?". The answer is "neither, because both are wrong".

[dhayden]

The way to handle this depends on whether you need to detect the error right away or of you can wait. To detect it right away:

    for (int r = 0; r < 6 && cin; r++) {
        for (int c = 0; c < 7 && cin; c++) {
            cin >> pizzaMatrix[r][c];
        }
    }
    if (!cin) {
        cout << "error\n";
        return 1;
    }

Note that I added braces around the for loops. Get in the habit of always using braces. It will prevent bugs.

If you can wait until the end of the loops then just let the extraction fail each time and keep the check at the very end:

    for (int r = 0; r < 6; r++) {
        for (int c = 0; c < 7; c++) {
            cin >> pizzaMatrix[r][c];
        }
    }
    if (!cin) {
        cout << "error\n";
        return 1;
    }

Which way should you do it? It depends on the program. If you don't mind having zeros stored in part of the matrix if it fails, then why have the extra checks? If you're actually going to be reading 100 million of these things, then it would be better to check earlier rather than wait and wait, only to find that it failed early on. Also, in that case you'd be able to say that if failed at line 4 of matrix 46,232,448. That would make it easier to fix the problem rather than saying "there's an error somewhere in that gigabyte file. Good luck finding it."