Simple read a double array from file

Question

I have double numbers in a file (one on each line) that I am trying to read into a c++ array. I am using the below code, but get the below error while running:

segmentation fault: 11

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
using namespace std;

int main () {
    string line;
    ifstream myfile ("temp2.csv");
    std::vector<double> myArray;
    int index = 0;
    if (myfile.is_open())
    {
        while (! myfile.eof() )
        {
            getline (myfile,line);
            cout << line << endl;
//            myArray[index++] << line;
            myArray[index++] = atoi( line.c_str() );
        }
        myfile.close();
    }

    else cout << "Unable to open file";

    return 0;
}

Please note that `while (!myfile.eof())` is considered [bad practice](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). — Rakete1111, Sep 22 '16 at 15:17
Why can I not find a dupe for this?!? The problem is your vector is empty. — NathanOliver, Sep 22 '16 at 15:19
@Rakete1111 - it's not "considered bad practice"; it's simply wrong. — Pete Becker, Sep 22 '16 at 15:26
@PeteBecker Oops, yeah you're right :) Thanks for mentioning — Rakete1111, Sep 22 '16 at 15:27
Code with off-by-one bugs may be OK, depending on your intentions. But it's certainly bad practice! — anatolyg, Sep 22 '16 at 15:30
@PeteBecker why is it **simply wrong**? It reads all lines. I agree with Rakete's first comment that it's bad practice. — eerorika, Sep 22 '16 at 15:32
@user2079303 - it executes the loop an extra time. Follow the link in the comment by Rakete1111. — Pete Becker, Sep 22 '16 at 15:42
@PeteBecker can you demonstrate how the OP's program loops an extra time? Here is my attempt, but I couldn't reproduce: http://coliru.stacked-crooked.com/a/bb0b0b8b53ea8864 input is two lines, the loop executes twice. I've seen the linked question, it's about a slightly different case. — eerorika, Sep 22 '16 at 15:47
@user2079303 - pretend that the file is empty. Follow the code: the loop executes once, even though there is no input. Then extrapolate. — Pete Becker, Sep 22 '16 at 15:51
@PeteBecker ah, it produces an empty line when the input is zero lines. I see how that case could be considered broken, though it might be sometimes desirable too: You could wish to interpret the input being a single line containing only EOF, in which case 1 loop would be correct. I don't get your "then extrapolate" comment. — eerorika, Sep 22 '16 at 15:56
@user2079303 It will work incorrectly if the last line in the file is empty (this is common with text files). Unfortunately, this is only clearly stated in a [comment](http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong#comment22561896_5605159). — anatolyg, Sep 22 '16 at 16:05
@anatolyg thanks for clarifying. So, the non eof loop condition would be better because it conveniently ignores the last blank line (if any). Still, I wouldn't be ready to call eof condition "simply wrong". The mistake is the lack of validating the input. OP's code assumes that every line really does start with a number. If the input validation is in place, then the iteration with the final empty line would simply fail the validation - just line any other empty lines. — eerorika, Sep 22 '16 at 16:22

score 2 · Answer 1 · edited Sep 22 '16 at 15:36

You can't do

myArray[index++] = atoi( line.c_str() );

It is an empty vector. You either need to push_back elements into it. Or initialize it with sufficient memory.

This should work:

#include <iostream>
#include <fstream>
#include <string>
#include <vector>
#include <stdlib.h>
using namespace std;

int main () {
    string line;
    ifstream myfile ("temp2.csv");
    std::vector<double> myArray;
    int index = 0;
    if (myfile.is_open())
    {
        while (! myfile.eof() )
        {
            getline (myfile,line);
            cout << line << endl;
//            myArray[index++] << line;
            myArray.push_back(atoi( line.c_str() ));
        }
        myfile.close();
    }

    else cout << "Unable to open file";

    return 0;
}

score 1 · Answer 2 · answered Sep 22 '16 at 15:19

On this line:

std::vector<double> myArray;

You create a vector using the default constructor. As you can see from the documentation, the default constructor creates an empty vector.

On this line:

myArray[index++] = atoi( line.c_str() );

You access elements from the vector with successively increasing indices. But those elements do not exist and the indices are out of bounds because the vector is empty. Accessing outside the bounds of the vector has undefined behaviour.

TL;DR you forgot to add any elements into the vector.

score 1 · Answer 3 · answered Sep 22 '16 at 15:25

1

The code is far more complex than it needs to be. It's much simpler to just read one value at a time:

std::vector<double> myArray;
double value;
std::ifstream myfile(temp2.csv);
if (!myfile) {
    std::cout << "Unable to open file\n");
    return EXIT_FAILURE;
}
while (myfile >> value)
    myArray.push_back(value);

answered Sep 22 '16 at 15:25

Pete Becker

74,985
8
76
165

It seems that OP wants to read only one value per line, and ignore the rest. This code reads all values, and also expects that they are separated by spaces/tabs (while the file format is CSV). – anatolyg Sep 22 '16 at 15:28
@anatolyg - the question not say "read only one value per line and ignore the rest". That's a guess you're making from reading the non-working code. Further, note that the question does not say "file format is CSV". Again, that's a guess you've made based on the file extension. Not an unreasonable guess, but coding decisions should not be based on guesses. – Pete Becker Sep 22 '16 at 15:46

Simple read a double array from file

3 Answers3