CODEWITHSUNDEEP
c++stl
Avinash
Avinash

Reputation: 13257

vector push_back crashing

I have following structure.

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

struct station {
    std::string id;
    std::string code;
    std::string station_name;
    station(std::vector<std::string> &in) : id(in[0]), code(in[1]), 
                                            station_name(in[2]) {}
    station(): id (""), code (""), station_name(""){}

    bool operator<( const station& rhs ) const {
        return this->station_name < rhs.station_name;   
    }
};
int main(int argc, char **argv) {    
    std::ifstream ifs(argv[1]);
    if ( ifs.peek() == EOF )  {
        exit ( 1 );
    }
    // Read the input file and update the database
    std::string line;
    station prev, current;
    std::set<station> my_set;
    while( ifs.good()&& std::getline(ifs,line) ) {
        std::stringstream  lineStream(line);
        std::string        token;
        std::vector<std::string> input;        
        while(std::getline(lineStream,token,',')) {
            input.push_back(token);
        }
        station st(input);
        my_set.insert(st);

    }
}

I am reading a file which has information related to railway stations in the following format ID,Station Code,Station Name

I am reading this file line by line and creating a object of station and then pushing the same into the std::set<station>

It gets crashed after some time, around after reading 21448 line. I have around 403523 lines

What is the problem here.

This program works properly on Linux but not on windows

I get debug assertion failed

Upvotes: 1

Views: 2646

Answers (3)

shibumi
shibumi

Reputation: 378

On what platform are your input files created? C++ run-time environment should decide what is the optimal line ending convention for your platform. So there might be some issue with that, but that doesn't explain why does the program wait till 21448 lines to crash.

And your error is not informative enough. Usually when an assertion fails, it shows where it failed. In your case, it seems like it could be failing in std::vector. Try replacing the [] operator with at() in your constructor. It throws an out-of-range exception when you try to access an invalid index.

Lastly, as others have already pointed out, it is not a good idea to pass a vector to a constructor especially, if you only have three arguments. Not only it might produce hard-to-catch bugs, but you are paying the price for an additional constructor and destructor in each iteration of the while loop. Hope it helps.

Upvotes: 0

Mark Ransom
Mark Ransom

Reputation: 308382

This is where it's handy to know some debugging techniques. In the spot where you create a new station, I'd put an assertion:

assert(station_init_vector.size() >= 3);

This makes sure you aren't accessing vector elements that don't exist.

An alternate tool is to use the vector member at() instead of the operator [], which does run-time index checking and will throw an exception for out-of-bounds indexing:

station(std::vector<std::string> &in) : id(in.at(0)), code(in.at(1)),
                                        station_name(in.at(2)) {}

Upvotes: 2

Loki Astari
Loki Astari

Reputation: 264551

The constructor that takes an array worries my.

Edit

Based on the updated question:

The problem is the constructor that takes a vector.
You are accessing the elements without check if they exist.

So if any line of input is bad (ie not all values are present) then the vector will not be as big as required and result in undefined behavior.

If you change these lines:

while(std::getline(lineStream,token,',')) {
            input.push_back(token);
        }

Into:

std::getline(linestream, id,   ',');
std::getline(linestream, code, ',');
std::getline(linestream, name, ',');

Then called the station constructor with these parameters (as would have been nice in OO code). Then you would not have seen the crash. Some error checking would also be nice.

Note:

while( ifs.good()&& std::getline(ifs,line) ) {

There is no need to check good() here. If the stream is not in a good state then getline() will do nothing. The conversion of the stream (the return value of getline()) to a bool is also checking if the state is the stream is valid for further reading and if not will convert to false (eventually).

// This is more idiomatic.
while(std::getline(ifs,line) ) {

Original

try this:

struct station
{
    std::string id;
    std::string code;
    std::string station_name;
    friend std::istream& operator>>(std::istream& stream, station& data)
    {
        std::string  line;
        std::getline(stream, line);

        std::stringstream  linestream(line);
        std::getline(linestream, data.id, ',');
        std::getline(linestream, data.code, ',');
        std::getline(linestream, data.station_name);

        return stream;
    }
};

int main()
{
    std::ifstream   file("station.txt");
    std::vector<station>  stations;

    std::copy(std::istream_iterator<station>(file),
              std::istream_iterator<station>(),
              std::back_inserter(stations)
             );
}

Upvotes: 2

Related Questions