2

Simple question; what's better and why?



    out.resize( in.size() );
    T1::iterator outit = out.begin();
    for( inIt = in.begin() to end, ++inIt, ++outIt )
       *outit = *inIt
OR

    out.erase();
    for( inIt = in.begin() to end, ++inIt )
        out.push_back( inIt );

I'm assuming the memory assignment implicit in push_back is worth avoiding but want to make sure.

Thanks

EDIT: Thanks for the out = in suggestions guys ;). The actual code I'm playing with is:


template//can't stop the browser ignoring th class T1, class T2 in angle brackets
bool asciihex( T1& out, const T2& in )
{
    //out.erase();
    out.resize( in.size() / 2 );
    if( std::distance( in.begin(), in.end() ) % 2 )//use distance rather than size to minimise the requirements on T2?
        return false;
    for( T2::const_iterator it = in.begin(); it != in.end(); it += 2 )
    {
        out.push_back(((( (*it > '9' ? *it - 0x07 : *it)  - 0x30)  '9' ? *(it+1) - 0x07 : *(it+1)) - 0x30) & 0x000f));
    }
    return true;
}

template
bool asciihex( T1& out, const T2& in )
{
    size_t size = in.size();
    if( size % 2 )//use distance rather than size to minimise the requirements on T2?
        return false;
    out.resize( size / 2 );
    T1::iterator outit = out.begin();
    for( T2::const_iterator it = in.begin(); it != in.end(); it += 2, ++outit )
    {
        *outit = ((( (*it > '9' ? *it - 0x07 : *it)  - 0x30)  '9' ? *(it+1) - 0x07 : *(it+1)) - 0x30) & 0x000f);
    }
    return true;
}

Edit: I've marked push_back as the answer as it seems to be the consensus and, therefore, more useful to anyone else with the same problem. However I have ended up using the iterator method as one of the container classes I'm interested in doesn't support push_back... mileage varies.

Patrick
  • 8,175
  • 7
  • 56
  • 72

7 Answers7

4

The second, and if you're concerned about multiple extensions use out.reserve(). The right answer to adding to a vector is almost always push_back or back_inserter, which avoid some possible problems (exception guarantees, constructors, writing past the end, for example) that you'd have to pay attention to with other methods.

David Thornley
  • 56,304
  • 9
  • 91
  • 158
2

The second one, as long as you reserve the right capacity first.

One problem I see (apart from style) is that in the first one, if your copy assignment throws, you have taken an operation that should give you the strong guarantee, and used it to give no guarantee.

Greg Rogers
  • 35,641
  • 17
  • 67
  • 94
2

If you care about preserving in then do:

out = in;

If you don't, then do:

std::swap(out, in);

If out and in are different types of containers, then try this:

out.erase(out.begin(), out.end());
// if it's a vector or other contiguous memory container, you'll want to reserve first
// out.reserve(in.size());
std::copy(in.begin(), in.end(), back_inserter(out));
Eclipse
  • 44,851
  • 20
  • 112
  • 171
1

So what's wrong with this?

out = in;

Don't you think that'd have the best possible behavior? If it doesn't, that sucks.

Also, your two code examples should be

out.resize( in.size() );
T1::iterator outIt = out.begin();
for( T1::iterator inIt = in.begin(); inIt != in.end(); ++inIt, ++outIt )
   *outIt = *inIt;

and

out.erase(out.begin(), out.end());
for( T1::iterator inIt = in.begin(); inIt != in.end(); ++inIt )
    out.push_back( *inIt );
Matt Cruikshank
  • 2,932
  • 21
  • 24
1

If you are dealing with simple values like integers or doubles it will not matter (performance wise) if you resize and set or reserve and push_back.

If you are dealing with more complex objects that have a significant constructor it is better to use the second push_back based method.

If you objects do not have a meaningful default constructor then the push_back approach is the only technique that works.

Jeroen Dirks
  • 7,705
  • 12
  • 50
  • 70
  • Good, clear answer. It's probably worth stressing that that reserve() isn't in the original question. That's the key to your answer. – Drew Dormann Dec 19 '08 at 16:56
1

I had a situation where I got worse performance in push_back-based solution compared to one using the [] operator on a resized vector. The cause of the difference was the overhead in checking whether the vector needs to expand its capacity every time it was doing a push_back.

What made it even worse was that the compiler decided it couldn't inline the call to push_back, but it did inline the reserve-logic into the push_back method. This gave the overhead of a function call and the function invoked was unnecessary large. Even after some force_inline massage, it wasn't as fast as the loop based on the [] operator.

A loop based on iterators or the [] operator won't do anything but writing the value, at least not when working with simple types. If working with more advanced types, the cost of the default constructor when resizing the vector may ruin the potential wins.

Anyway, if your program spends most of its time in these kinds of loop I definitely think you should benchmark the different solutions and see if there are any performance to gain by using one of the solutions. If you are not spending considerable amount of your execution time in the function you described, you shouldn't care about this post :)

Laserallan
  • 11,072
  • 10
  • 46
  • 67
0

I prefer push_back over array assignment as push_back will work with or without randomaccessiterators. Array assignment requires them. If you force randomaccessiterators now you can't easily change the container in the future. Of course, to avoid resize issues you call vector::capacity instead of resize.

jmucchiello
  • 18,754
  • 7
  • 41
  • 61