I wrote some array wrapper classes in C++. They work pretty well. However, I noticed that there is a lot of repetitive code. I was wondering if there was a good way to solve this problem. The CRTP looks like it could be used to eliminate all the redundant begins and ends, but I've never tried it before.

[source lang='cpp']
#pragma once
#include "ints.h"
#include "macros.h"

template <typename T, u32 N>
class Array
{
T arr[N];

public:

T& operator[](u32 i) {assert(i < size()); return arr;}
const T& operator[](u32 i) const {assert(i < size()); return arr;}
static u32 size() {return N;}

typedef T* iterator;
typedef const T* const_iterator;
const T* begin() const {return arr;}
const T* end() const {return arr+size();}
T* begin() {return arr;}
T* end() {return arr+size();}
};

template <typename T, u32 N>
class ArrayList
{
T arr[N];
u32 m_size;

public:
ArrayList() : m_size(0) {}

T& operator[](u32 i) {assert(i < size()); return arr;}
const T& operator[](u32 i) const {assert(i < size()); return arr;}

u32 size() const {return m_size;}
u32 capacity() const {return N;}

void clear() {m_size = 0;}

void add(const T& val)
{
assert(m_size < N);
if (m_size < N) {arr[m_size++] = val;}
}

void swapremove(u32 i) // last element is swapped, instead of moving all elements to preserve ordering
{
assert(i < size() && 0 < size());
if (m_size > 1) {arr = arr[m_size - 1];}
--m_size;
}

typedef T* iterator;
typedef const T* const_iterator;
const T* begin() const {return arr;}
const T* end() const {return arr+size();}
T* begin() {return arr;}
T* end() {return arr+size();}

//These functions are useful for adding elements in place
//T& next() {assert(m_size < N); return arr[m_size];}
//void sizeinc() {m_size++;}
};

template <typename T>
struct ArrayConstPtr
{
const T* arr;
u32 N;

//T& operator[](u32 i) {assert(i < size()); return arr;}
const T& operator[](u32 i) const {assert(i < size()); return arr;}
u32 size() const {return N;}

typedef const T* iterator; //for some reason this is required, even for constant iteration
typedef const T* const_iterator;
const T* begin() const {return arr;}
const T* end() const {return arr+size();}
};

[/source]

I'm not sure I understand your objection to this code or why you think the CRTP is a useful tool for "fixing" it...

Well code duplication is usually seen as a bad thing. But then again, using a trick to avoid it will decrease readability, which is also bad.

Have you tried this?

#include <array>

or, if your compiler only supports C++03,

#include <tr1/array>

That does good job of eliminating code duplication, too.

That would only replace one of the three and it doesn't have the same error checking behavior either. Plus I'm not sure if it would even work at all with exceptions disabled.

Inheritance?

Well ArrayConstPtr needs to be a POD type so virtual functions are out. I guess the most effective option might actually be *shudder* macros.

Let me upgrade my original statement.


Your objection to this "code duplication" is completely wrong and you should stop trying to "fix" it.


Think about it. You are not duplicating code; you have several unrelated pieces of code which happen to look very similar.

An Array is not an ArrayList is not an ArrayConstPtr. These are all distinct concepts. The fact that you want to iterate across all three is a little weird, and for what reason you want "wrappers" for these constructs is not at all clear to me, but whatever.

Going out of your way to fold them into a single code construct would actually be bad. It would suggest that there is a lot more in common between these three concepts than there really is. In fact, all you have is a coincidence, not a code duplication.

The more likely direction is that you will want richer wrappers, in which case you should probably emulate the standard C++ library and split them into multiple separate headers, each with its own set of support classes, free functions, etc. If you've never read a C++ standard library implementation, now might be a good time. Do you see them going out of their way to "eliminate" "duplication" like the fact that every SC++L container has a begin/end iterator fetch method?

Of course not.

Because the fact that these concepts are all iteratable is a coincidence of design, not a duplication of code.

Ok thanks. I was just curious whether people thought this was an example that could/should be refactored and I guess not. The current code works fine.