Why am I getting a timeout(using sstream to parse list of integers separated by commas)?

Question

I am given an input string of n integers, separated by commas (e.g. "23,4,56"). I need to set a stringstream to represent this string and then use it to scan each integer into a vector. The elements of the vector (the integers in the list) will end up being output line by line. I am given the main(), and am simply responsible for writing parseInts(string str). For some reason, I keep getting a timeout. I'm guessing it's something in my while loops, specifically concerning how I am manipulating my sstream with str(), but I can't figure out exactly what is going on. I am new to sstream and C++ so any help would be appreciated!

#include <sstream>
#include <vector>
#include <iostream>
using namespace std;

vector<int> parseInts(string str) {
    int a; //will use this to hold the value of the 1st int in the sstream
    stringstream list_initial; //will iterate over this sstream
    list_initial.str(str); //set sstream to represent input str
    vector<int> list_final; //will return this final vector for output in main
    while (!list_initial.str().empty()){ //stop iterating at end of string
        list_initial>>a; //store leading int value in a
        list_final.push_back(a); //add a to end of vector
        while (!ispunct(list_initial.str()[0])){ //get to next int in list
            list_initial.str(list_initial.str().erase(0,1));
        };
        list_initial.str(list_initial.str().erase(0,1)); //erase leading comma
    };
    return list_final;   
};

int main() {
    string str;
    cin >> str;
    vector<int> integers = parseInts(str);
    for(int i = 0; i < integers.size(); i++) {
        cout << integers[i] << "\n";
    }

    return 0;
}

What do you mean by 'timeout'? If your code is running slow, I suspect it is because of all the std::strings you are creating. A std::stringstream is a stream just like any other, and you should treat it as such. You don't need to keep creating a str()ing or resetting the stream's str()ing. — Dúthomhas, Nov 25 '15 at 23:16
What do you mean by this? I don't really understand how a stream works. I think ideally I could write something that just uses stream member functions that read in the list, ignoring the commas, and stopping at the end of the list. How do I do this? — Yang-Yang Feng, Nov 29 '15 at 00:56
You are constantly modifying the stream by creating and destroying new strings. You don't need to do that. See below for code to demonstrate. — Dúthomhas, Nov 29 '15 at 01:57

yolenoyer · Answer 1 · 2015-11-26T01:35:54.323

Your function implementation could be really improved, but following your logic, if you replace the second while line:

        while (!ispunct(list_initial.str()[0])){ //get to next int in list

by this one, adding a length check:

        while (list_initial.str().size() && !ispunct(list_initial.str()[0])){ //get to next int in list

, then is runs, works well and exits well.

Explanations

This loop was never breaking, because at the end of the process, ispunct() function was never recognizing its parameter, !list_initial.str()[0], as something true : the string list_initial.str() is empty at this moment, then the 0 index doesn't exist.
Please look at the doc on cplusplus.com:

If pos is equal to the string length, the const-version never throws exceptions (no-throw guarantee).
Otherwise, it causes undefined behavior.

Your program was freezing because it didn't found the right condition which could have leaded to leaving the above loop.

Advices

One important thing to talk about your question is : why didn't you find the answer yourself ? One answer for this question is the following : you didn't try to debug your code.

To answer your question, I just debugged your code, and found quickly the problem just by adding this kind of lines around your code, to see what happened:

cout << "list_initial: " << list_initial.str() << endl;

An example for your code, with debug statements added::

#include <sstream>
#include <vector>
#include <iostream>
using namespace std;

vector<int> parseInts(string str) {
    cout << "begin parseInts()" << endl;  // ######  DEBUG  ######

    int a; //will use this to hold the value of the 1st int in the sstream
    stringstream list_initial; //will iterate over this sstream
    list_initial.str(str); //set sstream to represent input str
    vector<int> list_final; //will return this final vector for output in main

    cout << "begin while 1" << endl;  // ######  DEBUG  ######
    while (!list_initial.str().empty()){ //stop iterating at end of string
        list_initial >> a; //store leading int value in a
        list_final.push_back(a); //add a to end of vector

        cout << "a: " << a << endl;  // ######  DEBUG  ######
        cout << "begin while 2" << endl;  // ######  DEBUG  ######
        while (!ispunct(list_initial.str()[0])){ //get to next int in list

            cout << "list_initial: " << list_initial.str() << endl;  // ######  DEBUG  ######

            list_initial.str(list_initial.str().erase(0,1));
        };
        cout << "endwhile 2" << endl;  // ######  DEBUG  ######

        list_initial.str(list_initial.str().erase(0,1)); //erase leading comma
    };
    cout << "endwhile 1" << endl;  // ######  DEBUG  ######

    cout << "end parseInts()" << endl;  // ######  DEBUG  ######

    return list_final;   
};

int main() {
    string str;
    cin >> str;
    vector<int> integers = parseInts(str);

    cout << "begin for" << endl;  // ######  DEBUG  ######
    for(int i = 0; i < integers.size(); i++) {
        cout << integers[i] << "\n";
    }
    cout << "end for" << endl;  // ######  DEBUG  ######

    return 0;
}

This (very basic) debugging technic permits you to quickly find out a bug in your code; just add some cout << ... << endl; lines inside the code to point out some important informations, i.e.: "Which loop is causing freezing?", or "What is the content of this variable at this moment?".

If you put cout << "Entering While loop 1" << endl; before, and cout << "Exiting While loop 1" << endl; after each suspected loop, you may learn many things about what happens during the execution of your program.

With this debugging lines added, you can easily find out that the second loop is looping forever; then you can narrow your bug investigation to this loop.

I see, thanks alot. I'm new to coding so the debugging tip is very helpful. You say that my function implementation could be improved, and I'm sure of this. I don't think I really understand how sstreams or streams in general work. Given an understanding of sstreams, what would be an elegant implementation? — Yang-Yang Feng, Nov 28 '15 at 23:22

score 0 · Answer 2 · answered Nov 29 '15 at 02:00

A stream is an abstract file. It is just a bunch of text that can be understood by things like, for example, cin >> n to convert numbers to integers.

Here's an idea of how to use a stream:

#include <cctype>
#include <ciso646>
#include <iostream>
#include <sstream>
#include <vector>
using namespace std;

vector<int> parse_ints( const string& s )
{
  vector<int> ints;     // resulting list of ints
  istringstream ss(s);  // stream of ints separated by (non-digits)
  int n;                // each int extracted from the stream

  // while input an int
  while (ss >> n)
  {
    // save the int
    ints.push_back(n);

    // skip to next digit
    while (ss and !isdigit(ss.peek())) ss.get();
  }

  return ints;
}

int main()
{
  vector<int> xs = parse_ints( "2, 3 ,5; 7 : 11.13\t17abc21" );
  for (int x : xs)
    cout << x << " ";
  cout << "\n";
}

This produces:

2 3 5 7 11 13 17 21

Notice how we can simply skip characters we aren't interested in? In this particular case, we skip all non-digit characters.

Hope this helps.

Why am I getting a timeout(using sstream to parse list of integers separated by commas)?

2 Answers2

Explanations

Advices