How to manage 'ostringstream' object in C++?

Question

This is a snippet of code from C++ program.

string TwoSeries::getArrays()
{
    ostringstream outIndex;
    ostringstream outValueA;
    ostringstream outValueB;
    string stA;
    string stB;
    string valueA;
    string index;
    int *arA;
    int * arB;
    string valueB;
    for(int x = 0; x < 200; x++)  
    {         

        outIndex << x;
        index = outIndex.str();



         arA = getArrayA();
        outValueA << *(arA + x);
        valueA = outValueA.str();


          arB = getArrayB();
        outValueB << *(arB + x);
        valueB = outValueB.str();


        stA += index + ":" + valueA + " ";
        stB += index + ":" + valueB + " ";

    }

   // return "Series A: \n"+stA+ "\n"+"Series B: \n"+ stB;   
    return index;
}

This function should return the last index converted from int into string, and that should be 199. But instead this object 'outIndex' concatenates all the digits (strings) into one string and gives as the result something like this: 1234567891011121314151617 ... 198199. And definitely the last number is 199. And what to force the function after full loop to output only the last number instead all the numbers it has come across. How to do this?

What do you think `ostringstream` does? Why do you insert values you don't want? — K-ballo, Nov 07 '11 at 21:59

sehe · Accepted Answer · 2011-11-07T22:10:47.460

2

You want to clear the string streams:

for(int x = 0; x < 200; x++)  
{         
    outIndex.str("");
    outValueA.str("");
    outValueB.str("");

Alternatively, you can adopt good C++ style and declare them locally in the loop:

for(int x = 0; x < 200; x++)  
{         
    ostringstream outIndex;
    ostringstream outValueA;
    ostringstream outValueB;

While you're at it you can move the rest as well. Or... rewrite as follows:

string TwoSeries::getArrays()
{
    string index;

    int x;
    for(x = 0; x < 200; x++)  
    {         
        ostringstream osA, osB;

        osA << x << ":" << *(getArrayA() + x) + " ";
        osB << x << ":" << *(getArrayB() + x) + " ";

        string stA = osA.str(); // warning: value isn't used
        string stB = osB.str(); // warning: value isn't used
    }

    ostringstream osA, osB;
    outIndex << (x-1); // previous index
    return outIndex.str();
}

Note that you're doing a lot of redundant work, and right now all those values aren't being used. Perhaps you have more code that you haven't shown :)

edited Nov 07 '11 at 22:10

answered Nov 07 '11 at 22:05

sehe

374,641
47
450
633

1

Why's declaring them in the loop "good C++ style"? Clearing them is more efficient, no? – rubenvb Nov 07 '11 at 22:10
Defining all variables at the top is old C style (pre C99, even). C++ idioms rely heavily on managing lifetimes of objects with a view to RAII (deterministic destruction), variable scope and exception handling. Updated my answer, by the way – sehe Nov 07 '11 at 22:12
sehe: might be great premature optimization, but I seem to remember a lot of performance related questions here on SO related to reconstructing a stream each time it was necessary vs just reusing (and thus clearing) one persistent object. Kind of unrelated, but still, important to keep in mind. C++ is not mindless pattern programming. – rubenvb Nov 07 '11 at 22:14
Well doh? :) Mindless pattern programming is when you do things 'because you seem to remember' things. Probably a good point, there @rubenvb, but the level of the question is not nearly in the realm of optimizing reallocations, if you ask me. – sehe Nov 07 '11 at 22:16

score 1 · Answer 2 · answered Nov 07 '11 at 22:06

Move objects that are only needed in the loop into the loop. This causes them to be reset each iteration:

string TwoSeries::getArrays()
{
    string stA;
    string stB;
    for(int x = 0; x < 200; x++)  
    {
        ostringstream outIndex;  //this stream used all three times. 
        outIndex << x;
        string index = outIndex.str();

        int *arA;
        arA = getArrayA();
        outIndex << *(arA + x);
        string valueA = outIndex.str();

        int * arB;
        arB = getArrayB();
        outIndex << *(arB + x);
        string valueB = outIndex.str();

        stA += index + ":" + valueA + " ";
        stB += index + ":" + valueB + " ";
    }

   return "Series A: \n"+stA+ "\n"+"Series B: \n"+ stB;   
}

Your problem was that each iteration you added the index to outIndex, but never reset it, causing it to slowly build up a list of all indexes ever used. This would also occur for your other two stringstreams. .str() does not clear the stream.

Thanks people, that was a quick answer and so simple solution. — ucas, Nov 07 '11 at 22:16

score 1 · Answer 3 · answered Nov 07 '11 at 22:10

for(int x = 0; x < 200; x++)  
{         
    outIndex << x;
}

Will continuously concatenate x onto outIndex. I think you need to do the following:

for(int x = 0; x < 200; x++)  
{         
    outIndex << x;

    ....

    outIndex.str("");
}

This will clear outIndex every time through the loop.

How to manage 'ostringstream' object in C++?

3 Answers3