0

I need to store an array of integers from a text file, but I can't find what I need to do for it exactly. I think I have the basis of the code set up, but I think I need to convert the elements into integers or something?

My output is my list :

50 0 20 10 18 -5 15 22 34 -1

But my "sorted" list ends up being -1, and a series of large negative numbers. I troubleshooted it to being something wrong with the array.

#include <iostream>
#include <fstream>
using namespace std;
int main() 
{
    int array1[30];
    int counter=0,n=0;
    fstream datafile;
    void bubbleSort(int list[], int length);
    void selectionSort(int list[], int length);
    /////////////
    datafile.open("data.txt");
    if (!datafile.is_open())
    {
        cout << "Failure to open." << endl;
        return 0;
    }
    while (!datafile.eof()) {
        datafile >> array1[counter];
        n++;
        cout << array1[counter] << endl;
    }
    datafile.close();
    //////////////////////////////
    //bubbleSort(array1, n);
    //selectionSort(array1, n);
    for (int i = 0; i < n; i++)
        cout << array1[i] << ", ";

    system("pause");
    return 0;
}
Joe Miller
  • 3,722
  • 1
  • 21
  • 38
Rey H
  • 3
  • 3
  • Did you mean for `n` and `counter` to be the same variable? – user253751 Apr 27 '16 at 04:11
  • Not sure if they technically are. I thought counter was the actual elements in the array and n was the length of the array. – Rey H Apr 27 '16 at 04:12
  • Well right now, counter always holds 0, and n holds the number of elements read so far. And then you read into the counter'th element, i.e. always the 0th because counter is always 0. So the rest of the elements hold garbage. – user253751 Apr 27 '16 at 04:43

2 Answers2

1

Never use eof(), since it leads to wrong programs. See https://stackoverflow.com/a/5837670 for the reason.

while (n < 30 && datafile >> array1[n]) {
    cout << array1[n] << endl;
    n++;
}
{
    int excess;
    if (datafile >> excess) {
        cerr << "error: data file too large\n";
        return;
    }
}

That way, the n will be correct at the end of the program.

Community
  • 1
  • 1
Roland Illig
  • 37,193
  • 10
  • 75
  • 113
  • 1
    So just to conclude, I should do this instead to avoid endless loop scenarios? I made the adjustments and will use these in future programs. – Rey H Apr 28 '16 at 00:06
0

Your code is all good except :

 while (!datafile.eof()) {
        datafile >> array1[counter];
        n++;
        cout << array1[counter] << endl;
    }

It should be:

while (!datafile.eof()) {
        datafile >> array1[n];
        if ( datafile.fail() ) break; 
        cout << array1[n] << endl;
        n++;
    }

Only a single index variable(n) is required to parse/store into a 1D array. The increment statement n++ in a while should always be the last one so that you are working on the current element and not the next.

Code:

#include <iostream>
#include <fstream>
using namespace std;
int main() 
{
    int array1[30];
    int n=0;
    fstream datafile;
    void bubbleSort(int list[], int length);
    void selectionSort(int list[], int length);
    /////////////
    datafile.open("data.txt");
    if (!datafile.is_open())
    {
        cout << "Failure to open." << endl;
        return 0;
    }
    while (!datafile.eof()) {
        datafile >> array1[n];
        if (datafile.fail()) break;
        cout << array1[n] << endl;
        n++;
    }
    datafile.close();
    for (int i = 0; i < n; i++)
        cout << array1[i] << ", ";

    system("pause");
    return 0;
}
Martin York
  • 234,851
  • 74
  • 306
  • 532
Ani Menon
  • 23,084
  • 13
  • 81
  • 107
  • That fixed it. Would it be possible for you to offer an explanation to the way that works? Thank you, BTW. – Rey H Apr 27 '16 at 04:22
  • Yes, in the while look you keep reading till the end of file and store it in an array. `counter` is not required as `n` keeps track of the index. See to that you use the same index variable within the loop to fetch the same element(like to print the array1[n]) and also increment(n++) should always be the last statement in a while loop so that you are still working on the current element & not the next. – Ani Menon Apr 27 '16 at 04:26
  • Nope. Your use of `eof()` is wrong. The EOF is not set until you read past the end of file. The last successful read goes upto but not past. Thus the while condition will be true but the read will fail and then you still print the value. There is a big SO question about the subject. – Martin York Apr 27 '16 at 05:04
  • 1
    [Why is iostream::eof inside a loop condition considered wrong?](http://stackoverflow.com/q/5605125/14065) – Martin York Apr 27 '16 at 05:09
  • @AniMenon: Nope. You need to put the check just after the read. So you read then test. If the read worked then you can add it to the array. See: https://gist.github.com/Loki-Astari/a75640a3c41fd9f4d20bde25fad10572 – Martin York Apr 27 '16 at 05:29
  • 1
    @AniMenon: Fixed it for you. – Martin York Apr 27 '16 at 07:19