HiveBrain v1.2.0
Get Started
← Back to all entries
patterncppMinor

Array kept contiguous by swapping with the last element

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
lastthearraywithswappingelementcontiguouskept

Problem

I made a class that encapsulates the common pattern of keeping an array contiguous when an element is removed by swapping the last element into its place.

A few specific things I'm wondering about:

  • Is it unsafe for non-POD types (because of the memcpy)?



  • Is there a more efficient way to add a new object (so it doesn't have to be copied)?



  • Should I worry about code bloat from capacity being a template parameter (N)?



This was not written with C++11 in mind, except for a few little features supported by Visual Studio 2010 (like auto).

Edit for clarification: I'm using aligned memory instead of a simple array of T so that I have control over when the objects in the array are created and destroyed. I'm using a memcpy to avoid calling a constructor every time something is swapped.

SwapArray.h:

```
#ifndef INCLUDE_GUARD_78F4FE5C_3E57_44C4_9E42_2574051D5A18
#define INCLUDE_GUARD_78F4FE5C_3E57_44C4_9E42_2574051D5A18

#include
#include

namespace ktc
{

// an array that keeps all its elements contiguous by swapping removed elements
// with the last element of the array.
//
// add and remove are both O(1).
template
class SwapArray
{
public:

SwapArray();
~SwapArray();

// number of elements currently in the array.
unsigned int size() const;
// max size.
unsigned int capacity() const;

// add a value to the end.
void add( const T& val );
// remove the value at this index.
void rem( unsigned int i );

// remove all.
void clear();

T& operator[]( unsigned int i );
const T& operator[]( unsigned int i ) const;

template
void sort( Compare compare );

private:

typedef
typename boost::aligned_storage
::value
>::type
AlignedMem;

// aligned for type T.
AlignedMem m_alignedMem;

unsigned int m_size;

// return the data for the i-th element.

Solution

No need to rewrite what Yuushi already mentioned, so, I will add few observations:

You seem to be creating something like boost::static_vector with one exception: your rem (remove) is like swap(at(i), back()); pop_back() - which is faster than simple erase(), but will reorder items.

I suppose that you want your objects destroyed immediatelly, therefore std::array would not be an option.

I have only one objection about the code:

template 
char* SwapArray::memAt( unsigned int i )
{
    auto ct = static_cast*> (this);
    return const_cast ( ct->memAt(i) );
}

template 
const char* SwapArray::memAt(unsigned int i) const
{
    ktcAssert( i ( m_alignedMem.address() ) + (i * sizeof(T));
}


This const_cast pattern can be seen everywhere. I understand that it could be good not to duplicate big code, but in this case (when it is quite short), copy-pasting two lines and adapting would be better. I would not use const_cast here.

It is getting too long there in comments, so, I will try to give one example against the memcpy:

class example {
    int first;
    int second;
    int *which;
public:
    example(): first(1), second(2) { which = &first; }
    example(const example& src): first(src.first), second(src.second) {
        which = src.which == &src.first ? &first : &second; }
};


Now we have class that will work in std::vector but your memcpy will break it.

Code Snippets

template <typename T, unsigned int N>
char* SwapArray<T,N>::memAt( unsigned int i )
{
    auto ct = static_cast<const SwapArray<T,N>*> (this);
    return const_cast<char*> ( ct->memAt(i) );
}

template <typename T, unsigned int N>
const char* SwapArray<T,N>::memAt(unsigned int i) const
{
    ktcAssert( i < N );
    return static_cast<const char*>( m_alignedMem.address() ) + (i * sizeof(T));
}
class example {
    int first;
    int second;
    int *which;
public:
    example(): first(1), second(2) { which = &first; }
    example(const example& src): first(src.first), second(src.second) {
        which = src.which == &src.first ? &first : &second; }
};

Context

StackExchange Code Review Q#62491, answer score: 4

Revisions (0)

No revisions yet.