0

The file "Athlete info.txt" looks like this:

Peter Gab 2653 Kenya 127

Usain Bolt 6534 Jamaica 128

Bla Bla 2973 Bangladesh -1

Some Name 5182 India 129

What I expect my code to do is read the first string and assign it to the firstName array (for eg Peter is stored in firstName[0]), read the second string and assign it to lastName array ( for eg Gab is stored in lastName[0]) and so on..I've tried many different ways and even tried making it all string arrays but it doesn't work. If anyone can tell me what's wrong in the code or how to go about it, that would be great!

Thanks in advance!

void readInputFromFile()
    {
        ifstream inputData;
        inputData.open("Athlete info.txt");

        const int SIZE=50;
        char firstName[SIZE],
             lastName[SIZE],
             athleteNumber[SIZE],
             country[SIZE];
        int  athleteTime[SIZE];

        int numOfCharacters=0;

        if (inputData.is_open())
        {
            int i=0;

            while(!inputData.eof())
            {
                inputData >> firstName[i]; 
                inputData >> lastName[i]; 
                inputData >> athleteNumber[i];
                inputData >> country[i]; 
                inputData >> athleteTime[i];
                i++;
                numOfCharacters++;
            }

            for (int i=0; i < numOfCharacters; i++ )
            {
                cout << "First Name: " << firstName[i];
                cout << "Last name: " << lastName[i];
                cout << "AthleteNumber: " << athleteNumber[i];
                cout << "Country: " << country[i];
                cout << "Time taken: " << athleteTime[i];
                cout << endl;
            }

        }
        else
        {
            cout << "ERROR" << endl;
        }
        inputData.close();
    }
Nico
  • 201
  • 3
  • 11
  • `while(!inputData.eof())` http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong – drescherjm Dec 10 '16 at 16:24
  • You are reading 1 character. instead of a string. This code would be cleaner and better if you were permitted to use `c++`. I mean std::string instead of a c-array for the strings.. – drescherjm Dec 10 '16 at 16:28

2 Answers2

3

First of all you are using c++, so let's use std::string, and use classes. Lets create Athlete struct which contains everything your athlete needs:

struct Athlete {
    Athlete() = default;
    Athlete(std::stringstream &stream) {
        stream >> firstName 
               >> lastName 
               >> athleteNumber 
               >> country 
               >> athleteTime;
    }
    // every Athlete is unique, copying should be prohibited
    Athlete(const Athlete&) = delete;

    std::string firstName;
    std::string lastName;
    std::string athleteNumber;
    std::string country;
    std::string athleteTime;
}

Maybe you could work on this one a bit more and encapsulate it better.

now we will use std::vector to store Athletes and every time we push_back we will call Athelete constructor with read line from input file. Later on you can use range-based for loop to access every single Athlete in vector. Also note that ifstream doesn't to be closed manually, it is closed automatically as soon as object reaches out of scope.

void readInputFromFile() {
   ifstream inputData("Athlete info.txt");
   std::vector<Athlete> athletes;

   std::string line;
   std::stringstream ss;

   if (inputData.is_open() {
      while (getline(inputData, line)) {
         ss.str(line);
         // directly construct in-place
         athletes.emplace_back(ss);
      }

      for (const Athlete& a : athletes) {
         /* ...*/
      }
   } else {
      std:cerr << "ERROR" << std::endl;
   }
}
  • Great answer. I did not know you could pass a `std::stringstream` to `getline`. I'll be using this in my own code! – Jack Deeth Dec 10 '16 at 16:36
  • Thanks a lot! It contains a lot of what I haven't done yet, so I'm finding it a little confusing, but thanks nonetheless! – Jannatul Ferdoas Dec 11 '16 at 09:31
  • Don't worry to ask anything, i will gladly help to bring understanfin upon yourself – Marek Chocholáček Dec 11 '16 at 12:07
  • You've prohibited copying and perform `push_back` on `Athlete` object. This seems a bit controversial to me. I think you should use `emplace_back`. (Yes, I know the compiler will call the move constructor anyway, this just seems more logical). – Hedin Apr 29 '17 at 12:46
0

The simplest change would be

while(inputData >> firstName[i])
{
    inputData >> lastName[i]; 
    inputData >> athleteNumber[i];
    inputData >> country[i]; 
    inputData >> athleteTime[i];
    i++;
    numOfCharacters++;
}

This would rely on the input being correctly formatted.

Jack Deeth
  • 1,813
  • 16
  • 26
  • Thanks but this doesn't save the firstName Peter in firstName[0]. This is because when I later cout << firstName[0] I get weird values or nothing at all. – Jannatul Ferdoas Dec 11 '16 at 09:27
  • Hmm, that's strange! I'm sure I tested it and it worked… However @Marzi's answer is the better answer, if you're able to depart from the styles of old textbooks. – Jack Deeth Dec 11 '16 at 11:45