The discussion centers around issues with a C++ program that uses vectors to store employee information but experiences input problems within a do-while loop. The user encounters an issue where the program skips prompts for last names after the first entry, which is attributed to unhandled input buffer issues. Suggestions include using `cin.ignore()` to clear the input buffer after reading user input, which resolves the skipping problem. Additionally, there is a desire to implement EOF detection for future enhancements, with advice given to check the `eofbit` flag when reading from files. The conversation also touches on the potential use of iterators and class structures for a more scalable design in the user's project.
About the part that read the file two times. This is the problem I am trying to avoid. Here is what happened when I deleted the read before the do-while loop:I made the first file.read into comment line. I move the DirV.push_back below the file read in the loop.
This is the result, the last line is repeated.
I have to think of some other way to fix this if I have to eliminate one read. BUT still, that doesn't help reading an empty file. The first read will read empty file.
What are the problems reading an empty file? If just reading garbage, then it's not too bad. But if it can do other damage, then it's not good.
I have to experiment more with that, I did run into problem before. What will happen reading an empty file? Is it just reading garbage or can it do damage to the file?
Step through the code mentally, thinking through what each line will do. Let us pull that code section down here so that we can examine it more easily.
C++:
else
{//If the file exist, read entire file into vector DirV.
file.read(reinterpret_cast<char*>(&Dir), sizeof(Dir));
do
{
DirV.push_back(Dir);
file.read(reinterpret_cast<char*>(&Dir), sizeof(Dir));
} while (!file.eof());
file.close();
}
Line 3: we start by attempting to read from the file into an array element. But the file is empty. So the read fails and the end file.eof() flag is set.
Line 4-5: We enter a do while loop. Since this is a do-while, there is no initial test of the termination condition.
Line 6: We do a DirV.push_back(Dir). One supposes that this somehow commits an array item to the in-memory directory. Based on the name "push_back", this is likely a metaphorical stack push. But the array entry we are pushing is garbage -- the read had failed.
Line 7: We attempt to read a second record, oblivious to the fact that we've already hit end of file.
Line 8: We check the loop termination condition. [One hopes and expects that the EOF condition remains set after multiple attempts to read past end of file]
About the part that read the file two times. This is the problem I am trying to avoid. Here is what happened when I deleted the read before the do-while loop:
Rule: Think before you code. You want to figure out the failure modes when writing the code, not when running it.
Rule: Always check the status code. [This one I picked up in a course on VMS system programming in the 80s. It was advice well worth listening to].
You want the sequence of operations to be:
Read
Test for end of file (not yet)
Push
Read next
Test for end of file (not yet)
Push
...
Read next
Test for end of file (EOF - exit)
There are a lot of ways to put that into a while loop.
You could put one read outside the loop and use:
Code:
Read
while not EOF
Push
Read
end while
Thinking back to first year programming in the '70's, when structured programming was just becoming all the rage, this is pretty much what we were taught to do.
Or you could test for the exit condition mid-loop
Code:
while (true)
read
if EOF then break
push
end while
This has the advantage that you're not coding the read statement twice. And it puts the exit test where it belongs -- right when the exit condition arises.
I often find myself using the following construct - emulating a break.
Code:
DONE = false
while ( not DONE )
read
if not EOF
then
push
else
DONE = true
end if
end while
Where better to put this task in Constructor and Distructor.
In methods on the object with names like read_data and write_data.
You do not want to put code that does I/O into a constructor or destructor. The constructor and destructor should only be for allocating and deallocating memory for the object itself (which is done automatically by the compiler), not for reading data from disk or writing data to disk. If you try to read data from disk in an object's constructor, and something goes wrong, the object will be in an inconsistent state. If you try to write data to disk in an object's destructor, and something goes wrong, the object is gone and you have no way to recover from the error.
#56
yungman
5,741
294
PeterDonis said:
In methods on the object with names like read_data and write_data.
You do not want to put code that does I/O into a constructor or destructor. The constructor and destructor should only be for allocating and deallocating memory for the object itself (which is done automatically by the compiler), not for reading data from disk or writing data to disk. If you try to read data from disk in an object's constructor, and something goes wrong, the object will be in an inconsistent state. If you try to write data to disk in an object's destructor, and something goes wrong, the object is gone and you have no way to recover from the error.
Ah, I see what you mean about writing in Distructor, that if that fails, the program is closed. So I should use member function to write to file.
But what's wrong loading the file with Constructor? What do you mean the object will be in an inconsistent state?
what's wrong loading the file with Constructor? What do you mean the object will be in an inconsistent state?
If an error happens inside the constructor, there is no guarantee that the object will be constructed correctly. So you should not do anything inside a constructor that might cause an error.
The "C/C++ way" to do that is do...while. That executes at least once.
Possibly I am missing something. That would seem to mean that the exit condition needs to be checked twice.
Code:
do {
read
if not EOF
push
end if
} while not EOF
#60
yungman
5,741
294
jbriggs444 said:
Rule: Think before you code. You want to figure out the failure modes when writing the code, not when running it.
Rule: Always check the status code. [This one I picked up in a course on VMS system programming in the 80s. It was advice well worth listening to].
You want the sequence of operations to be:
Read
Test for end of file (not yet)
Push
Read next
Test for end of file (not yet)
Push
...
Read next
Test for end of file (EOF - exit)
There are a lot of ways to put that into a while loop.
You could put one read outside the loop and use:
Code:
Read
while not EOF
Push
Read
end while
Thinking back to first year programming in the '70's, when structured programming was just becoming all the rage, this is pretty much what we were taught to do.
Or you could test for the exit condition mid-loop
Code:
while (true)
read
if EOF then break
push
end while
This has the advantage that you're not coding the read statement twice. And it puts the exit test where it belongs -- right when the exit condition arises.
I often find myself using the following construct - emulating a break.
Code:
DONE = false
while ( not DONE )
read
if not EOF
then
push
else
DONE = true
end if
end while
I tried your suggestion:
It still print out the last line twice.
I wish I can find the thread that we talked about this, it's tricky. The eof() does not become true right away and double write the last data.
What is wrong with that? Logically, the condition for deciding to push the record is not necessarily the same as the condition to continue reading. They happen to be the same in this case, but they don't have to be.
#62
yungman
5,741
294
I have to do this to make it work. I remember last time when I have the issue, the eof() does not become true upon reading the last element, it's one after the last element. But it will still contain the last element in the buffer. So the next push_back, it will repeat writing the last element again.
The following way can fix it:
We spent a lot of time on this, it's so important that I wrote down clearly in my notes.
Unless there is any reason against this way, I am going to put this in my program. This will definitely avoid reading empty file.
It has been stated at least three times by three different people that you should NOT be doing file I/O in the constructor, and same for the destructor.
#64
yungman
5,741
294
Mark44 said:
It has been stated at least three times by three different people that you should NOT be doing file I/O in the constructor, and same for the destructor.
I know, give me time to change the program, want to resolve the double read first. I still need to change the names also.
Then I can delete the Constructor and Distructor, can't think of any reason to keep them.
I just created readFile() and writeFile() and copy the Constructor and Distructor into them resp. Then write code in main() to call readFile() at the beginning and call writeFile at the end of the program.
Last edited:
#65
yungman
5,741
294
I fixed the program, file reading and writing are out of the constructor and distructor. In fact, I got rid of the constructor and distructor. I don't see any use of them. This is a running program.
I change the names to more readable. I don't need to have double read in line 30 after I move the while(!file.eof()) to the top instead of do-while().
This is the .h file
C++:
#ifndef VectorCall_H
#define VectorCall_H
#include <vector>
#include <fstream>
class DirectoryClass
{
private:
const static int nameLen = 25, phoneLen = 12;
public:
struct Directory { char name[nameLen]; char phone[phoneLen]; };
std::vector<Directory>DirV;
Directory Dir; bool newF, failOpen;
std::fstream file;
bool readFile()//
{
failOpen = false;
file.open("Directory.dat", std::ios::in | std::ios::binary);
if (file.fail())
{//If file doesn't exist, create the file
std::cout << " Fail to open file.\n";
file.open("Directory.dat", std::ios::out | std::ios::binary | std::ios::app);
file.close();
failOpen = true;
}
else
{//If the file exist, read entire file into vector DirV.
while (!file.eof())
{
file.read(reinterpret_cast<char*>(&Dir), sizeof(Dir));
if (file.eof()) { break; }
DirV.push_back(Dir);
};
file.close();
}
return failOpen;
}
void createFile()
{ file.open("Directory.dat", std::ios::out | std::ios::binary | std::ios::app);
file.close(); }
void writeFile()
{
int ct2 = 0;
file.open("Directory.dat", std::ios::out | std::ios::binary);
do//write the updated data in vector DirV into the file before closing
{
file.write(reinterpret_cast<char*>(&DirV[ct2]), sizeof(DirV[ct2]));
ct2++;
} while (ct2 < DirV.size());
file.close();
}
void push_backV() { DirV.push_back(Dir); }
void print() { std::cout << "Name: " << Dir.name <<
" Phone: " << Dir.phone << "\n\n"; }
};
#endif
This is the source.cpp
C++:
#include <iostream>
#include <string>
#include <vector>
#include "VectorCall.h"
using namespace std;
int main()
{
char more; int index = 0; const int nameLen = 25, phoneLen = 12;
DirectoryClass Information;
char displayAll, tryAgain = false;
Information.readFile();
if (Information.failOpen == true)
{cout << " Fail to open file, want to try again? "; cin >> tryAgain;}
if (tryAgain == true) Information.readFile();
if (Information.failOpen == true)
{//First time set up the directory by entering all the names and information
cout << " File does not exist, let's create the file. " <<
"You need to enter informations:\n\n";
Information.createFile();
cin.ignore();
do
{ cout << " Enter name: "; cin.getline(Information.Dir.name, nameLen);
cout << " Enter phone: "; cin.getline(Information.Dir.phone, phoneLen);
//Info.print();
Information.push_backV();//writing into vector DirV.
cout << " Do you want to enter another name? "; cin.get(more);
cin.ignore();
} while (tolower(more) == 'y');
}
//Ask whether to display all the information in the file.
cout << " Press 'y' to see the whole list. "; cin >> displayAll;
cin.ignore(); cout << endl;
if (displayAll == tolower('y'))
{ do
{ cout << "Name stored in DirV is: " << Information.DirV[index].name <<
" phone: " << Information.DirV[index].phone << "\n\n";
index++;
} while (index < Information.DirV.size());
}
else { cout << " Bye\n\n"; }
Information.writeFile();
return 0;
}
Hope this looks better. Let me know what else.I was looking at the next program in the book, I notice even with the Declaration and Implementation files, the book shows using a lot of internal functions within the main program body. I would think it's better to push as much to external as possible.
I understand Class is a blueprint to create object incidence. But I keep thinking it's more useful to put as much work ( functions) into the Specification and Implementation files as possible so the main() don't have to do a lot. It's like calling external functions to do the job. Isn't it the point that try to use external function( or whatever name you call) to do as much as possible so you can make the main() as simple as possible for fast turn around? It's like structure, define the structure and use it to make lives easier.
I can envision having a class to read and write file, using arguments like 1) c-string that contains the filename, path where to save or to open the file, 2) the kind of file and 3) the vector to write to the file like what I am doing. So this Class can read and write different files with different ADT datatype.
Then another class to define structure, cin information, store into a vector like what I am doing. Within that, I can put add element, delete element, sort elements.
With that, the main() just call them to do all the jobs. So far, things are still short, I use inline function. But I am sure when I add in the add, delete and sort, I have to have a separate Implementation file to avoid a very long declaration file.
What are the problems reading an empty file? If just reading garbage, then it's not too bad. But if it can do other damage, then it's not good.
If you try to read from an empty file, you reach end-of-file immediately. I tested this with the following program:
C++:
#include <iostream>
#include <fstream>
#include <iomanip>
using namespace std;
int main ()
{
// Open a file for output, but don't write anything to it.
// Then close it.
ofstream outputFile ("emptyfile.dat");
cout << "Trying to open emptyfile.dat for output..." << endl;
if (outputFile)
{
cout << "Successfully opened emptyfile.dat for output." << endl;
}
else
{
cout << "Failed to open emptyfile.dat for output." << endl;
}
outputFile.close();
cout << "Closed emptyfile.dat." << endl;
// Open that file for input, and try to read from it.
cout << "Trying to open emptyfile.dat for input..." << endl;
ifstream inputFile ("emptyfile.dat");
if (inputFile)
{
cout << "Successfully opened emptyfile.dat for input." << endl;
cout << "Trying to read from emptyfile.dat..." << endl;
string line;
if (getline (inputFile, line))
{
cout << "'" << line << "' was read from emptyfile.dat." << endl;
}
else
{
cout << "EOF on emptyfile.dat." << endl;
}
inputFile.close();
cout << "Closed emptyfile.dat." << endl;
}
else
{
cout << "Can't open emptyfile.dat." << endl;
}
return 0;
}
Output:
Code:
Trying to open emptyfile.dat for output...
Successfully opened emptyfile.dat for output.
Closed emptyfile.dat.
Trying to open emptyfile.dat for input...
Successfully opened emptyfile.dat for input.
Trying to read from emptyfile.dat...
EOF on emptyfile.dat.
Closed emptyfile.dat.
#67
yungman
5,741
294
I am so glad I posted my program in post 65! I can't find my original program now that I am ready to get back to it! I have to copy both files from post 65 to recreate the program! That really save my day.
I finally finish the other part of the program in sorting, adding, deleting, now back to the Class and files.