Simple multi-dimensional Array class in C++11

The new version of the code can be reviewed in Simple multi-dimensional Array class in C++11 - follow-up.

The following code implements a simple multi-dimensional array class (hyper_array::array).

I modeled most (if not all) the feature on the orca_array.hpp header by Pramod Gupta after watching his talk at this year's cppcon.

I think that orca_array is fine. However, a more generic implementation might prove interesting as well (e.g. reducing code repetition, gaining more efficiency through compile-time computations/verification and allowing more dimensions (even though the relevance of the latter feature is debatable)).

The element type and the number of dimensions is given at compile-time. The length along each dimension is specified at run-time.

As a start, there are 2 configuration options:

• HYPER_ARRAY_CONFIG_Check_Bounds: controls run-time checking of index bounds,

• HYPER_ARRAY_CONFIG_Overload_Stream_Operator: enables/disables the overloading of operator




The implementation requires some C++11 features and uses very basic template (meta)programming and constexpr computation when possible.

I have mainly the following goals:

• Self-contained (no dependency to external libraries), single-header implementation

• Minimal code repetition (if any) and clarity/"readability" of implementation

• Conforming to "good" programming practices in modern C++ while developing a solution that might prove interesting to use for others

• Clean API "that makes sense" to the user

• As much compile-time computation/evaluation/input validation as possible (template-metaprogramming, constexpr?)






• Maximum efficiency while remaining written in standard C++11 (I made an exception once for std::make_unique(), but I'll probably remove it),

• Allow inclusion in STL containers while still being efficient

My current concerns are:

• I'm sure that many computations could be done and for` loops could be unwound at compile time, but I haven't wrapped my head a

I think your class isn't nearly as useful enough as it could be due to your choices in member variables. Furthermore, it's less efficient than it could be. Also the code is written in a style that overcomplicates the problem.

Member Variables

Your members are:

std::unique_ptr
ElementType* const
const std::array
const SizeType
const std::array

This choice makes the class noncopyable and nonassignable. But why? There's nothing inherent about a multidimensional array that suggests it shouldn't be assignable or copyable. You make some members public. There's no reason to do that. Particularly bad is data - which is redundant with _dataOwner.

You should strive to make your class as generic as possible. To that end I suggest you simply have two members, both private:

ElementType* data;
std::array dimensions;

You can derive dataLength and indexCoeffs from dimensions if need be, and since you'd have to iterate over the array to do anything anyway, I don't see what precomputing saves you.

This also allows you do support copying and moving.

Forwarding References

Forwarding references are a great choice for function templates when you can take objects by lvalue or rvalue and do the cheapest correct thing possible in all cases. However, everywhere that you are using them, the objects getting past in must be integral types (I don't see you checking this, but you should). There is no different between copying and moving an integral type, so simply take everything by value. That saves you from having to do all of the std::forward<>-ing. For example:

template 
ElementType& at(Indices... indices)
{
    return data[rawIndex(indices...)];
}

Bounds Checking

You introduce a macro for whether or not to do bounds checking. However, convention from the standard library suggests that we just provide functions that DO range checking and functions that don't. at() should throw std::out_of_range, and operator() should never throw:

template 
ElementType& operator()(Indices... indices) {
    // nothrow implementation
}

template 
ElementType& at(Indices... indices)
{
    some_range_checking(indices...);
    return (*this)(indices...);
}

Compile time checking

First, a cleaner way to write are_integral would be to use the bool_pack trick:

template  struct bool_pack { };

template 
using all_true = std::is_same, bool_pack>;

With:

template 
using are_all_integral = all_true::value...>;

And you should actually use that metafunction as part of the signature of every function! That is preferred to a simple static_assert since any reflection-style operations on your class would actually yield the correct result:

template ::value && sizeof...(DimensionLengths) == Dimensions>
          >
array(DimensionLengths... )
{ ... }

Otherwise, you would get something weird like std::is_constructible, std::string> being true.

Iterators

An important part of writing a container is writing iterators for it. You may just provide general iterators that just go over the whole array front to end. Or you may want to support arrays that iterate over a single dimension and provide a proxy object to a multi-dimensional array of one dimension less. Either would be good to have.

StackExchange Code Review Q#107877, answer score: 2