You are going to drive yourself nuts with input errors if you don't nail down the file input aspect of filling your list. Why !.eof() inside a loop condition is always wrong. (it is and it is corrupting your attempts at input).
The problem with your use of while (!info.eof()) is after reading last line, .eof() is NOT yet set, so you loop again attempting, info >> newNode->id; which fails setting eofbit. However, since you don't check the stream state after each read, you invoke Undefined Behavior in your code continually attempting to read from a stream with eofbit set for name, address, country, number and imagePath. The game is over at that point. Your code could appear to operate correctly or SEGFAULT or anything in between. It's operation is no longer defined.
However, your data corruption starts with the very first six-lines of data for the very first node. Since you are reading groups of 6-lines of data for each node, you must make every read with getline(). Otherwise >> will leave the '\n' at the end of each line Unread.
Your next call to getline() for name reads nothing because it found '\n' as the next character in the input stream and considered the input an empty-line. getline() extracts the '\n' and returns. You then read name into address, and address into country. Your next info >> newNode->number; fails because your line ordering is now off and you attempt to read "japan" into number which fails setting failbit on the stream (which isn't checked and isn't eofbit) so the carnage continues... completely corrupting each node you attempt to add to your list.
C++ provides a much simpler and robust way to handle input. When working with any class or struct, you can overload the >> (std::istream) and << (std::ostream) operators to read just what is needed to fill your personalInfo struct. With classes, you declare the overload as a friend function of the class to provide access to the private members. In your case you could declare:
public:
...
/* overload istream for input and ostream for output */
friend std::istream& operator >> (std::istream& is, PersonalInfo::personalInfo& pi);
friend std::ostream& operator << (std::ostream& os, PersonalInfo::personalInfo& pi);
and then define the >> operator to read and VALIDATE 6-lines of input at-a-time filling a personalInfo object. You would write that as:
std::istream& operator >> (std::istream& is, PersonalInfo::personalInfo& pi)
{
std::string id{}, name{}, address{}, country{}, number{}, imagePath{};
/* validate read of six-lines of input */
if (getline(is, id) && getline(is, name) && getline(is, address) &&
getline(is, country) && getline(is, number) && getline(is, imagePath)) {
/* convert integer values with std::stoi, initialize tmp strucct */
PersonalInfo::personalInfo tmp { std::stoi(id), name, address, country,
std::stoi(number), imagePath, nullptr };
pi = tmp; /* assign to pi reference */
}
return is;
}
(note: the friend keyword is only used on the declaration inside the class, not in the definition that follows if defined separately. Also note, you could assign each value to pi.member directly, but it was easier to just initialize a tmp struct and assign to pi)
You should add a try {...} catch {...} around the initialization of tmp above to catch and handle any error in the conversion of std::stoi(id) or std::stoi(number). If a failure in conversion occurs, you would want to set failbit for the stream state of is above so the read loop terminates at the first failure and does not attempt to add the invalid data. You can use is.setstate (std::ios_base::failbit); to set failbit on the stream in the catch portion of your try-block. I'll leave that to you.
(see the example at the link above, essentially you just wrap the initialization of tmp and assignment to pi in the try portion and output an error setting failbit in the catch clause)
With the overload defined, you simply loop reading a personalInfo object at a time and adding it to your linked list, e.g.
void PersonalInfo::readFile()
{
if (filename.empty()) { /* validate filename not empty */
std::cerr << "error: readFile() - filename empty.\n";
return;
}
PersonalInfo::personalInfo tmp{}; /* temporary struct to fill */
std::ifstream f(filename); /* open file */
if (!f.is_open()) { /* validate file open for reading */
std::cerr << "error: readFile() - file open failed " << filename << '\n';
return;
}
while (f >> tmp) /* read six lines at a time filling tmp */
add (tmp); /* allocate and initialize node with tmp */
}
The add() function just isolates the allocation and initialization of the node and adding it in-order to your list. You can do it all in readFile(), but I find it cleaner to not combine the I/O readFile() with the list-operation add(), e.g.
bool PersonalInfo::add (const PersonalInfo::personalInfo& pi)
{
/* allocate and initialize node */
PersonalInfo::personalInfo *node = new PersonalInfo::personalInfo;
*node = pi;
if (!Head) /* add to list */
Head = Tail = node;
else {
Tail->next = node;
Tail = node;
}
return true;
}
(note: Tail always points to the last node in the list allowing adding nodes to the end of the list in O(1) time. For the first node, both Head and Tail point to the first node. Thereafter, you simply update Tail->next to the address of the new node and then reset Tail to that address)
To output (print) your list you do much the same overloading the << operator so it outputs one node in your list, e.g.
std::ostream& operator << (std::ostream& os, PersonalInfo::personalInfo& pi)
{
os << "\nid : " << pi.id << '\n'
<< "name : " << pi.name << '\n'
<< "address : " << pi.address << '\n'
<< "country : " << pi.country << '\n'
<< "number : " << pi.number << '\n'
<< "imagePath : " << pi.imagePath << '\n';
return os;
}
You can change the formatting of the output to anything you like. A simple prnList() function that simply iterates over each node in the list outputting each node can be written as:
void PersonalInfo::prnList()
{
PersonalInfo::personalInfo *iter = Head;
while (iter) {
std::cout << *iter;
iter = iter->next;
}
}
Avoid using MagicNumbers and hardcoding filenames in your code. You shouldn't have to recompile just to read from a different filename. Provide the filename to your object as part of the constructor. You can use a single constructor and provide an empty filename if none is given when the class is constructed. (You would just need to write another member function, a "setter" function to set the filename). You can write the constructor as follows:
PersonalInfo (std::string fname="") { Head = Tail = nullptr; filename = fname; }
Which optionally takes a std::string argument for the filename, initializing filename empty if none is provided.
main() takes arguments. That is what int argc, char **argv are for. A simple way of providing the filename to read is just to provide it as the first argument to your program and use argv[1] to initialize the filename in your PersonalInfo class when you declare it. That makes setting the filename when you create the class as simple as:
int main (int argc, char **argv) {
if (argc < 2) {
std::cerr << "error: insufficient input\n"
"usage: ./program filename\n";
return 1;
}
PersonalInfo P { argv[1] };
P.readFile();
P.prnList();
}
I'll have to look back and see if there are other major points that need addressing. Putting it altogether, a short example implementation of your list would be:
#include <iostream>
#include <fstream>
#include <string>
class PersonalInfo {
private:
struct personalInfo {
int id;
std::string name;
std::string address;
std::string country;
int number;
std::string imagePath;
struct personalInfo* next;
};
personalInfo *Head, *Tail; /* add Tail pointer for O(1) in-order insertion */
std::string filename;
public:
PersonalInfo (std::string fname="") { Head = Tail = nullptr; filename = fname; }
/* overload istream for input and ostream for output */
friend std::istream& operator >> (std::istream& is, PersonalInfo::personalInfo& pi);
friend std::ostream& operator << (std::ostream& os, PersonalInfo::personalInfo& pi);
/* add to allocate and intialize each node */
bool add (const PersonalInfo::personalInfo& pi);
void readFile();
void prnList();
};
std::istream& operator >> (std::istream& is, PersonalInfo::personalInfo& pi)
{
std::string id{}, name{}, address{}, country{}, number{}, imagePath{};
/* validate read of six-lines of input */
if (getline(is, id) && getline(is, name) && getline(is, address) &&
getline(is, country) && getline(is, number) && getline(is, imagePath)) {
/* convert integer values with std::stoi, initialize tmp strucct */
PersonalInfo::personalInfo tmp { std::stoi(id), name, address, country,
std::stoi(number), imagePath, nullptr };
pi = tmp; /* assign to pi reference */
}
return is;
}
std::ostream& operator << (std::ostream& os, PersonalInfo::personalInfo& pi)
{
os << "\nid : " << pi.id << '\n'
<< "name : " << pi.name << '\n'
<< "address : " << pi.address << '\n'
<< "country : " << pi.country << '\n'
<< "number : " << pi.number << '\n'
<< "imagePath : " << pi.imagePath << '\n';
return os;
}
bool PersonalInfo::add (const PersonalInfo::personalInfo& pi)
{
/* allocate and initialize node */
PersonalInfo::personalInfo *node = new PersonalInfo::personalInfo;
*node = pi;
if (!Head) /* add to list */
Head = Tail = node;
else {
Tail->next = node;
Tail = node;
}
return true;
}
void PersonalInfo::readFile()
{
if (filename.empty()) { /* validate filename not empty */
std::cerr << "error: readFile() - filename empty.\n";
return;
}
PersonalInfo::personalInfo tmp{}; /* temporary struct to fill */
std::ifstream f(filename); /* open file */
if (!f.is_open()) { /* validate file open for reading */
std::cerr << "error: readFile() - file open failed " << filename << '\n';
return;
}
while (f >> tmp) /* read six lines at a time filling tmp */
add (tmp); /* allocate and initialize node with tmp */
}
void PersonalInfo::prnList()
{
PersonalInfo::personalInfo *iter = Head;
while (iter) {
std::cout << *iter;
iter = iter->next;
}
}
int main (int argc, char **argv) {
if (argc < 2) {
std::cerr << "error: insufficient input\n"
"usage: ./program filename\n";
return 1;
}
PersonalInfo P { argv[1] };
P.readFile();
P.prnList();
}
Example Use/Output
With your sample data in the file named dat/personalinfo.txt, you would run the program as follows receiving the following output:
$ ./bin/ll_personalinfo dat/personalinfo.txt
id : 1
name : Sendai
address : 2-chome
country : japan
number : 110
imagePath : C:\\info.jpg
id : 2
name : John Douglass
address : 2nd block
country : america
number : 911
imagePath : C:\\info2.jpg
The key in all of this is to nail down your input routine. Validate all six-lines are read and that every member of personalInfo is filled before considering the input valid. You should also enclose ech std::stoi() conversion in a try {..} catch {..} block to catch any errors during conversion to int. That I leave that to you.
Look things over and let me know if you have further questions.
