1

I am trying to read integers from a file to an array, but when I try to read the elements of said array, I get numbers that have little to nothing to do with the numbers in the file.

#include <fstream>
#include <istream>
#include <iterator>
#include <string>
using namespace std;

//Main Function
int main()
{
    //Declare variables and ask user for file names
    string f1, f2;
    int s1, s2, n = 0;
    cout<<"Please enter the names of the files you would like to merge, file extensions included."<<endl;
    cout<<"File 1 Name: ";
    cin>>f1;
    cout<<"File 2 Name: ";
    cin>>f2;

    //Opening files
    ifstream fs1, fs2;
    fs1.open(f1);
    fs2.open(f2);

    //Checking if both files exist
    if (fs1 && fs2)
    {
        //Getting length of files
        s1 = distance(istream_iterator<int>(fs1), istream_iterator<int>());
        s2 = distance(istream_iterator<int>(fs2), istream_iterator<int>());

        //Declaring arrays and writing values to them
        int a1[s1], a2[s2];
        while(!fs1.eof())
        {
            fs2 >> a2[n];
            n++;
        }

        //Closing files
        fs1.close();
        fs2.close();

        //Reading array
        for (int i=0; i<s2; i++)
        {
            cout<<a2[i]<<endl;
        }
    }

    //If the requested files do not exist
    else
    {
        cout<<"No file exists with that name."<<endl;
    }

}

I have two text files, file1.txt and file2.txt. The code opens the files, determines the number of integers within, and creates an array with the number of integers.

file1.txt: 45 69 87 3 9 32 11 9 6 66

file2.txt: 4

The output I get when reading the array for file1.txt (a1[]) gives very unexpected numbers:

File 1 Name: file1.txt
File 2 Name: file2.txt
0
0
1878276352
6421656
4
6422016
3756032
48
16
4199922

As you can see, this is not the output I expected. file2.txt only consists of the number 4, and its ouput was simply 16

I am somewhat new to c++ and programming in general, so you might have to bear with me. Does anybody see what I've done wrong?

I am using Code::Blocks and gcc is my compiler. Thank you for your time.

Alexis
  • 13
  • 2
  • 3
    `while(!fs1.eof()) { fs2 >> a2[n]; ... }` Doesn't this look suspicious to you? – Shane Bishop Jan 18 '21 at 21:20
  • 2
    @ShaneBishop is on to something - for two reasons.: 1) You read from the wrong stream. 2) It will try to read one value more than you have available in the stream - and you'll use that non-read value later. Don't use `while(!stream.eof()) { stream >> variable; }` use `while(stream >> variable) { ... }` – Ted Lyngmo Jan 18 '21 at 21:24
  • 1
    What is `n` when you're done with your `while` loop? I think you're getting EOF sooner than you expect. – jkb Jan 18 '21 at 21:28
  • Also note that `int a1[s1], a2[s2];` isn't standard C++. More on that here: [Why aren't variable-length arrays part of the C++ standard?](https://stackoverflow.com/questions/1887097/why-arent-variable-length-arrays-part-of-the-c-standard) - In short: Use `std::vector a1(s1), a2(a2);` instead. – Ted Lyngmo Jan 18 '21 at 21:28
  • In C++ you should declare variables in the scope they're used - directly before usage (like `n`, `s1`, `s2`), it improves readability. This is not `C` where you had to declare them at the top. – bloody Jan 18 '21 at 21:47
  • 1
    Unrelated: Probably cheaper to [`std::vector::push_back`](https://en.cppreference.com/w/cpp/container/vector/push_back) or use [`std::back_inserter`](https://en.cppreference.com/w/cpp/iterator/back_inserter) and put up with the reallocations than read the file twice. Come to think of it, you probably need to rewind the files after reading to the end to get the lengths before you can read the numbers from the file. – user4581301 Jan 18 '21 at 21:55

2 Answers2

2

Tested, using std:

std::vector<int> read_file(std::string filename) {
    int n;
    std::vector<int> vector;
    std::ifstream file;
    file.open(filename);

    if (file.is_open()) {
        std::string line;
        while (std::getline(file, line)) {
            std::istringstream is(line);
            while (is >> n) {
                vector.push_back(n);
            }
        }
        file.close();
    }

    return vector;
}

int main() {
    std::vector<int> vector = read_file("file1.txt");

    for (auto element: vector) {
        std::cout << element << std::endl;
    }
}
Pedro Rio
  • 56
  • 1
  • 6
  • While I didn't end up using this method, you helped me better understand vectors. Later in the project I actually used a vector, so thank you! I'll be sure to use vectors more in the future. – Alexis Jan 19 '21 at 17:09
1

Note: int a1[s1], a2[s2]; doesn't comply with the ISO C++ standard. For details, see Why aren't variable-length arrays part of the C++ standard? If you cannot know the length of an array prior to runtime, consider using std::vector.

It is as @user4581301 says, you need to rewind the files, since std::distance() will read the files to the end.

Otherwise you will already be at the end of the files, and you won't read any data into your arrays, which means your arrays will be holding uninitialized memory.

Here is a solution:

// Rewind the files
fs1.clear();
fs2.clear();
fs1.seekg(0);
fs2.seekg(0);

// Read into array 1
for (n = 0; fs1 >> a1[n]; ++n) {}

// Read into array 2
for (n = 0; fs2 >> a2[n]; ++n) {}

seekg() sets the position in the stream to the given offset. seekg(0) resets to the beginning of the stream.

clear() clears the stream's internal error state flags, including the eofbit.

Shane Bishop
  • 1,708
  • 1
  • 7
  • 20