Looping through strings in c++

Question

I have text file which has n words. I am trying to read it and print using c++. But, I am able to print only last string. Please help me

int main()
{
    ifstream inFile;
    inFile.open("test.txt");

    if (inFile.fail()) {

        cerr << "Error opening file" << endl;

        exit(1);
    }

    string x;
    string a[100];
    int count = 0, i = 0;

    while (!inFile.eof()) {
        inFile >> x;
        a[i] = x;
        count++;
    }

    for (i = 0; i < 100; i++) {
        cout << a[i];
    }

    return 0;
}

Answer 1

Your bug is that you forgot to increment i in the loop:

while (!inFile.eof()) {
    inFile >> x;
    a[i] = x;

    // You don't increment the `i`
    // That is why you only see the last word.

    count++;
}

But that is not enough there is another bug that was previously hidden.

while (!inFile.eof()) {  // The last read read up to but not past the eof
                         // So the eof() is still false but there are no
                         // words left in the file.

    inFile >> x;         // So this read will fail

    a[i++] = x;          // Even though your read failed you still
                         // Add it to the array.

    count++;
}

Let us fix this bug by testing to see if the read worked.

while (!inFile.eof()) {
    if (inFile >> x) {
        a[i++] = x;

        count++;
    }
}

Here you can see we test the state of the stream after the read. But this puts all the meaningful code inside the if branch. Why not just push this out one level.

while (inFile >> x) {
    a[i++] = x;
    count++;
}

Here we read a word from the file. If it works enter the loop, otherwise don't.

That should fix all the bugs for small files, but it is worth going through the rest of the file to look for bad practice and improvements.

// You don't show your headers.
// You should include them.
// Also through the code you don't use `std::` objects with the prefix. This means
// your code contains a `using namespace std;` I know all the books use this
// but it is considered exceedingly bad practice in real code. You should
// avoid it (even for small programs as the habit may cause you to use it
// large programs).
int main()
{
    // Not sure why you want to take two lines here?
    // There is a constructor that takes a file name.
    ifstream inFile;
    inFile.open("test.txt");


    // The stream object when used by itself in a boolean context
    // (i.e. when used in an if expression like here. Will convert itself
    // to bool checking the current state. So you don't need to explicitly
    // call the `fail()` method.
    if (inFile.fail()) {

        // It's not going to make much difference here.
        // But in general you should prefer to use '\n' unless you 
        // absolutely must force a flush of the stream (which generally is no
        // as the stream will flush itself at the most appropriate time). In
        // this case it is also useless as `cerr` is unbuffered and `exit()`
        // would force a flush of any other buffers.
        cerr << "Error opening file" << endl;

        exit(1);
    }

    string x;
    // You only ever have 100 words in the input file?
    // Arrays are not resizable and force the initialization of all members.
    // So this will either be too little space and you will overflow (or need
    // to add specific code to check for overflow and prevent it) or it is
    // way too much space and you have needlessly constructed all those objects.
    //
    // For dynamic array like storage prefer std::vector
    string a[100];

    // Come on. Every coding standard in the world says one variable per line.
    // A lot of people are fine with short variables like `i` but I prefer
    // to use meaningful names for all variables. Especially if the code
    // may grow in the future.
    int count = 0, i = 0;

    // This `while (!inFile.eof())` is WRONG and an anti-pattern.
    //
    // Testing for `eof` here will find the EOF after the you
    // have read the last word from the file. Remember the last
    // read will read up to but not past the eof. So when your read
    // in the loop reads the last word `eof()` is still false.
    // So now you will re-enter the loop and try and read past the
    // end and the read will fail. Since the read fails `x` is
    // unchanged and the last word will be added to `a` twice.
    //
    // The correct way to do this is to read in the while condition.
    //     while(inFile >> x) {
    //
    // This will read a word from the file and return the stream to
    // be used in a bool context (which will be true if you read a
    // word correctly from the stream). So what this does is try
    // and read a word if it works enter the loop if it fails (i.e.
    // you reach the end of the file) the loop will not be entered
    // as the stream object returned is in a bad state with eof set
    while (!inFile.eof()) {
        inFile >> x;   // read a string
        a[i] = x;      // Copy the string
                       // You can do this in a single line
                       // inFile >> a[i]

        // You don't increment the `i`
        // That is why you only see the last word.

        // Whats the difference between `count` and `i`?
        count++;
    }
    // That was a nice loop but with a vector you can read the whole
    // file into the vector in a single line.
    // std::vector<std::string> a(std::istream_iterator<std::string>(inFile),
    //                            std::istream_iterator<std::string>());


    // Here `i` is normal and OK (single line loop variable).
    for (i = 0; i < 100; i++) {
        cout << a[i]; // No space between words?
    }

    // But you can declare it as part of the for loop and prevent it leaking
    // into the scope of the rest of your code.
    //     for(int i = 0; i < count; ++i) {
    //
    // Note 1: declare and initialize.
    // Note 2: the number of words read is not 100 but count.
    // Note 3: Prefer the prefix increment ++i
    //         It makes no difference here. But when using some objects it
    //         can. A lot of C++ code is changed simply by changing the type
    //         of the variables. So if you use the most efficient version of
    //         the code in all contexts then it will remain the most
    //         efficient even after maintenance.
    //
    // In C++ we usually loop over containers using begin() and end()
    // to get iterators. This make sit easy to convert to standard
    // algorithms if we need to. This is easier when you use a container
    // rather than an array as they maintain an accurate end. But it can
    // be done with arrays.
    //     for(auto loop = std::begin(a); loop != std::end(a); ++loop) {
    //
    // Also note that in modern C++ you can use `range based for`.
    // So it will find the begin and end of the array for you.
    //     for(auto const& obj: a) {
    //
    // More useful for std::vector than an array but worth mentioning.
    // It allows things like the trick I do with creating the vector
    // directly from the file as the vector can be constructed using
    // iterators and the iterator library has input iterators that
    // work on streams.


    // Not required in main()
    // If your application can not fail then prefer to not return
    // anything.
    return 0;
}

I would write it like this:

#include <string>
#include <fstream>
#include <iostream>
#include <iterator>
#include <vector>

int main()
{
    std::ifstream inFile("test.txt");

    if (!inFile) {
        std::cerr << "Error opening file" << "\n";
        exit(1);
    }

    std::vector<std::string> data(std::istream_iterator<std::string>(inFile),
                                  std::istream_iterator<std::string>());

    for(auto const& word: data) {
        std::cout << word << "\n";
    }
}

Answer 2

Another part that would require attention is your while loop.

while( !inFile.eof() ) {
        inFile >> x;            (2)
        a[i]=x;
        i++; 
   }

In this way, the last string of your input file will be written twice. This is because your check happens before you grab the last x (2). What you need to do is,

 while( inFile >> x ) {
            a[i++] = x;
            count ++;
       }

Lastly, what happens if there are more than 100 variables in the input file? Do you ignore them or have you thought of a way to deal with that? In the code you give, you will just get a segmentation fault.

Answer 3

You don't require to implement everything again. This is already implemented in C++ directory

#include <iostream>
#include <fstream>

int main()
{
    std::ifstream f("file.txt");

    if (f.is_open())
        std::cout << f.rdbuf();
}

Hope this works!!

Answer 4

On this line:

    a[i] = x;

You use i but it's never incremented. Instead do:

    a[i++] = x;

Answer 5

You need to increment i, as per RoQuOTriX's answer, but you probably also want to increment count so you know how many words you have. Also you should avoid over filling your array/

Try this:

int main() {

    ifstream inFile;
    inFile.open("test.txt");


    if(inFile.fail()) {

        cerr << "Error opening file"<< endl ;

        exit(1);
    }

    string x;
    string a[100];
    int count=0,i=0;

    while( (!inFile.eof()) && (count<100))   // avoid array overrun!
        inFile >> x;
        a[i]=x;
        i++; // i think you meant i++ not count++ (as per RoQuOTriX)
        count++;    // as per your original answer
    }

    for (i=0;i<count;i++){      //  only show values you actually read!
        cout<< a[i];
    }

    return 0;
}

Answer 6

You did not increment the i variable in your while loop, therefore assigning and overwriting always the first element:

int main() {

    ifstream inFile;
    inFile.open("test.txt");


    if(inFile.fail()) {

        cerr << "Error opening file"<< endl ;

        exit(1);
    }

    string x;
    string a[100];
    int count=0,i=0;

    while( !inFile.eof()) {
        inFile >> x;
        a[i]=x;
        i++; // i think you meant i++ not count++
    }

    for (i=0;i<100;i++){
        cout<< a[i];
    }

    return 0;
}