views:

396

answers:

5

I'm trying to build a string using data elements stored in a std::list, where I want commas placed only between the elements (ie, if elements are {A,B,C,D} in list, result string should be "A,B,C,D".

This code does not work:

typedef std::list< shared_ptr<EventDataItem> > DataItemList;
// ...
std::string Compose(DataItemList& dilList)
{
    std::stringstream ssDataSegment;
    for(iterItems = dilList.begin();
        iterItems != dilList.end(); 
        iterItems++)
    {
        // Lookahead in list to see if next element is end
        if((iterItems + 1) == dilList.end())  
        {
            ssDataSegment << (*iterItems)->ToString();
        }
        else
        {
            ssDataSegment << (*iterItems)->ToString() << ",";
        }
    }
    return ssDataSegment.str();
}

How do I get at "the-next-item" in a std::list using an iterator? I would expect that it's a linked-list, why can't I get at the next item?

+8  A: 

You cannot do it + N because you have no random access for list iterators. You can only do one step at a time with list iterators (these are bidirectional iterators).

You can use boost::next and boost::prior

// Lookahead in list to see if next element is end
if(boost::next(iterItems) == dilList.end())  
{

Or you can print the comma before:

std::string Compose(DataItemList& dilList)
{
    std::stringstream ssDataSegment;
    for(iterItems = dilList.begin();
        iterItems != dilList.end(); 
        ++iterItems)
    {
        if(iterItems != diList.begin())
            ssDataSegment << ",";
        ssDataSegment << (*iterItems)->ToString();
    }
    return ssDataSegment.str();
}
Johannes Schaub - litb
Note: next() and prev() are in boost/utility.hpp. Unless they move somewhere else.
sheepsimulator
The boost::next() solution is elegant. Many thanks.
sheepsimulator
Technically, it's boost::prior().
sheepsimulator
@sheepsimulator, thanks corrected.
Johannes Schaub - litb
+3  A: 

I believe that a list iterator is bidirectional, but not random access. That means that you can do ++ and -- to it but not add or subtract.

To get the next iterator, make a copy and increment it.

Zan Lynx
+1  A: 

You could avoid this problem altogether by using:

std::string Compose(DataItemList& dilList)
{
    std::stringstream ssDataSegment;
    for(iterItems = dilList.begin(); iterItems != dilList.end(); iterItems++)
    {
        ssDataSegment << (*iterItems)->ToString() << ","; // always write ","
    }
    std::string result = ssDataSegment.str();
    return result.substr(0, result.length()-1); // skip the last ","
}

You first write the "," for all elements (even for the last one). Than afterwards, you remove the unwanted last "," using substr. This additionally results in clearer code.

Danvil
Always +1 to the cleaner solution. ;-)
DevSolar
I disagree that this is clearer code. It's never clearer to do extraneous operations and then undo them later. Also, this is non-idiomatic, seeing as the idiomatic way is to treat the first iteration as special. -1
rmeador
+5  A: 

Another solution is to have the first entry be the special case, instead of the last entry:

std::string Compose(DataItemList& dilList)
{
    std::stringstream ssDataSegment;
    for(iterItems = dilList.begin();
        iterItems != dilList.end(); 
        ++iterItems)
    {
        // See if current element is the first
        if(iterItems == dilList.begin())  
        {
            ssDataSegment << (*iterItems)->ToString();
        }
        else
        {
            ssDataSegment << "," << (*iterItems)->ToString();
        }
    }
    return ssDataSegment.str();
}
Fred Larson
Alternate approach is make sure container not empty, print first item, increment iterator, and the loop just has your "else" print and no if/else needed.
Mark B
@Mark B: Yes, I saw Johannes did exactly that (I didn't see that in his answer when I posted mine). That's a bit tidier.
Fred Larson
+1  A: 

Yet another possibility:

#include "infix_iterator.h"
#include <sstream>

typedef std::list<shared_ptr<EventDataItem> > DataItemList;

std::string Compose(DataItemList const &diList) {
    std::ostringstream ret;
    infix_ostream_iterator out(ret, ",");

    for (item = diList.begin(); item != diList.end(); ++item)
        *out++ = (*item)->ToString();
    return ret.str();
}

You can get infix_iterator.h from Google's Usenet archive (or various web sites).

Jerry Coffin