6
\$\begingroup\$

I wrote this code to read from a CSV text file (containing data such as 12,3,568,48,3,8 with no more than 3 digits to each number). It stores the numbers as char arrays in the vector values. However, this does seem like a clumsy way of doing this with resizing the vector and copying the chars. Is there a neater/more succinct way of doing this?

#include <iostream>
#include <fstream>
#include <assert.h>
#include <vector>

int main() {
    using namespace std;
    ifstream in;
    in.open("Data.txt");
    assert(in.is_open());
    vector<char*> values;
    const int MAXSIZE = 4;
    char thisVal[MAXSIZE];
    while(in.getline(thisVal,MAXSIZE,',')) {
        values.resize(values.size() + 1);
        values.back() = new char[MAXSIZE];
        strcpy(values.back(), thisVal);
    }
    in.close();
    for (char* cp: values) cout<<*cp<<endl;
    return 0;
}
\$\endgroup\$
6
\$\begingroup\$

Storing char* in a vector leads to leaks. instead use std::string, which will manage its own buffer and delete it as needed.

vector<string> values;

inserting to the back of a vector can be done with push_back or you can construct in place with emplace_back in C++11:

while(in.getline(thisVal,MAXSIZE,',')) {
    string str(thisVal);
    values.push_back(std::move(str));
    //std::move signals that I won't be using str anymore and that the underlying buffer can just be passed through to the new element. 
    // After this str will not be valid. You can still assign to it normally but until you do any result of calling a method on it will be implementation defined.
}

or

while(in.getline(thisVal,MAXSIZE,',')) {
    values.emplace_back(thisVal);
}

Failing to open a file is common enough to use an explicit if and print an error message instead of asserting (which won't do anything when compiled as a release version):

if(!in.is_open()){
    cerr << "failed to open file.";
    return 1; 
}

I return 1 here to signal to the OS that the program didn't finish successfully and a script using it can then react to it.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.