16

Suppose you want to have a static array of pre-defined values/objects (const or non-const) associated with a class. Possible options are to use std:vector, std::array or C-style array (ie. []), or . For example,

In .hpp:

class MyClass {
public:
    static const std::vector<MyClass> vec_pre; // No efficient way to construct with initializer list, since it always uses Copy Contructor, even when using std::move
    static const std::array<MyClass, 2> arr_pre; // Have to specify size which is inconvenient
    static const MyClass carr_pre[]; // Not compatible with C++11 for-range since size is undefined
};

In .cpp

const std::vector<MyClass> MyClass::vec_pre = { std::move(MyClass{1,2,3}), std::move(MyClass{4,5,6})  }; // NOTE: This still uses copy constructor
const std::array<MyClass, 2> MyClass::arr_pre= { MyClass{1,2,3}, MyClass{4,5,6} };
const ZwSColour ZwSColour::carr_pre[] = {  MyClass{1,2,3}, MyClass{1,2,3} }

When writing this intially, I chose the std::vector since I don't have to specify the size, I get all the goodness of the vector class, and it seems like the modern C++ way to do it. PROBLEM: while testing, I noticed that it would call the Move contructor, but then still call the Copy constructor for each element. The reason for this is the std::initializer_list only allows const access to its members, and so the vector has to copy them from the initializer_list to its own storage. Even though it's only done once at startup, this is inefficient and there doesn't appear to be a way around it, so I looked at other options (std::array and C-array[]).

Second choice was to use std::array, which is also a modern C++ way, and it doesn't suffer from the problem of calling the Copy Constructor for each value since it doesn't need to create copies (Not sure why though exactly ?). std::array also has the benefit that you don't need to wrap each value in std::move(). However, it has the annoyance that you have to specify the size first, so every time you add/remove elements, you have to change the size as well. There are ways around this but none of them are ideal. As @Ricky65 states, you should just be able to do

std::array <int> arr = { 1, 3, 3, 7, 0, 4, 2, 0, 3, 1, 4, 1, 5, 9 }; //automatically deduces its size from the initializer list :)

This leaves me with the last option - the good old C-style array [] - which has the benefits that I don't have to specify the size, and is efficient in that it doesn't call the Copy constructor for each object. Downsides are that it's not really modern C++, and the biggest downside is that if you don't specify the size of the array in the .hpp header, then the C++11 for-range doesn't work as the compiler complains

Cannot use incomplete type 'const MyClass []' as a range

You can overcome this error by either specifying the size of the array in the header (but this is inconvenient and produces hard-to-maintain code, as you need to adjust the size every time you add/remove items from the initializer list), or alternatively use constexpr and completely declare the array and values in the .hpp header

constexpr static MyArray my_array[] = { MyClass{1,2,3}, MyClass{4,5,6} };

NOTE: The constexpr "work-around" will only work for POD's and so cannot be used in this case for Class objects. The above example will result in a compile-time error Invalid use of incomplete type 'MyClass'

I'm trying to write best-practice modern C++ where possible (eg. using copy-and-swap idiom), and so wonder what is the best way to define static arrays for a class...

  • without having to specify the size
  • which don't need to be Copy constructed (or Move constructed either, if possible)
  • which can be used with C++ for-range
  • which don't need to be specified in the header file
  • Should compile/work for Clang/LLVM 3.5, Visual Studio 2013 Update 4 RC, and GCC 4.8.1.

EDIT1: Another post about vector problem of not being able to move values from initializer list

EDIT2: More info on using std::array without the need to specify size, which also creates/uses make_array(), and mentions that there is a proposal for make_array() to become a a standard. Original SO link courtesy of comment by @Neil Kirk.

EDIT3: Another problem with the vector method (at least in this case) is that you cannot iterate over the items using a const T or T. It only allows iteration using const T& (when it's static const) and const T&/T& (when it's static). What's the reason for this limitation ?

Descriptive Answer to solutions

@Yakk's solution appears to be the only solution, and also works on Visual C++ 2013 Update 4 RC.

I find it staggering that such a trivial issue is so difficult to implement using the latest C++11/14 standard.

Community
  • 1
  • 1
DarkMatter
  • 306
  • 2
  • 13
  • 4
    "Have to specify size which is inconvenient" - see http://stackoverflow.com/questions/26351587/how-to-create-stdarray-with-initialization-list-without-providing-size-directl – Neil Kirk Oct 20 '14 at 01:37
  • 2
    `std::move(MyClass{1,2,3})` is superfluous because `MyClass{1,2,3}` is already an rvalue. – Oktalist Oct 20 '14 at 14:10
  • _`std::array` doesn't need to create copies (Not sure why though exactly?)_ Probably the implementation can just take ownership of the internal array of the `initializer_list` as an optimization. – Oktalist Oct 20 '14 at 14:39
  • @Oktalist - re. `std::move()` being superfluous - when it's used, the move constructor does get called, followed by the copy constructor. Why is it the move constructor even gets called if it's superfluous ? Shouldn't this be optimized away ? – DarkMatter Oct 20 '14 at 17:12
  • @Oktalist `std::vector` must use a copy constructor because it does not take ownership of the items in the `initializer_list`, where as `std::array` does take ownership of those items. [DarkMatter](http://stackoverflow.com/users/1998099/darkmatter)'s [link](http://stackoverflow.com/questions/24793019/stdvector-initialization-move-copy-constructor-of-the-element) explains this. – Jonathan Mee Oct 20 '14 at 18:06
  • @JonathanMee That link doesn't say anything about `std::array`. – Oktalist Oct 20 '14 at 21:12
  • @DarkMatter Not sure what you're asking. `std::vector{MyClass{1,2,3}}` first moves `MyClass{1,2,3}` into an `initializer_list`, then copies the `initializer_list` into the `vector`. Using `std::move` won't change that. – Oktalist Oct 20 '14 at 21:16
  • @Oktalist - If I put std::move(MyClass{1,2,3}) in the initializer list, the MyClass move contructor is called followed by the MyClass copy constructor. If I just put MyClass{1,2,3} in the initializer list, only the MyClass copy constructor is called. I understand why the copy constructor is called, as the vector needs a copy the intializer list item, but I don't understand why the move constructor is called with std::move(), when the compiler should be able to deduce that it can't do a move, and so should only call the copy constructor. – DarkMatter Oct 20 '14 at 21:24
  • @DarkMatter Ah, excellent question. Using `std::move` in this case means that the conditions are not met which would otherwise allow the compiler to elide a move constructor with side-effects (§12.8/31). – Oktalist Oct 20 '14 at 21:51
  • 1
    Your EDIT3 belongs in a separate question with its own MCVE. – Oktalist Oct 20 '14 at 23:25

3 Answers3

8

The data does not have to be stored within the class. In fact, storing the data within a static member of the class is leaking implementation details.

All you need expose is that the data is available, and that data is global to the class type. This does not involve exposing storage details: all you need to expose is storage access details.

In particular, you want to expose the ability to for(:) loop over the data, and operate on it in a C++11 style way. So expose exactly that.

Store the data in an anonymous namespace in the class's .cpp file in a C-style array (or std::array, I don't care).

Expose in the class the following:

namespace details {
  template<
    class R,
    class iterator_traits,
    class iterator_category,
    bool is_random_access=std::is_base_of<
        std::random_access_iterator_tag,
        iterator_category
    >::value
  >
  struct random_access_support {};
  template<class R, class iterator_traits, class iterator_category>
  struct random_access_support<R, iterator_traits, iterator_category, true> {
    R const* self() const { return static_cast<R const*>(this); }
    template<class S>
    typename iterator_traits::reference operator[](S&&s) const {
      return self()->begin()[std::forward<S>(s)];
    }
    std::size_t size() const { return self()->end()-self()->begin(); }
  };
}

template<class It>
struct range:details::random_access_support<
  range<It>,
  std::iterator_traits<It>,
  typename std::iterator_traits<It>::iterator_category
> {
  using value_type = typename std::iterator_traits<It>::value_type;
  using reference = typename std::iterator_traits<It>::reference;
  using iterator = It;
  using iterator_category = typename std::iterator_traits<It>::iterator_category;
  using pointer = typename std::iterator_traits<It>::pointer;

  It begin() const { return b; }
  It end() const { return e; }

  bool empty() const { return b==e; }
  reference front() const { return *b; }
  reference back() const { return *std::prev(e); }

  range( It s, It f ):b(s),e(f) {}

  range()=default;
  range(range const&)=default;
  range& operator=(range const&)=default;
private:
  It b; It e;
};

namespace details {
  template<class T>
  struct array_view_helper:range<T*> {
    using non_const_T = typename std::remove_const<T>::type;
    T* data() const { return this->begin(); }

    array_view_helper( array_view_helper const& ) = default;
    array_view_helper():range<T*>(nullptr, nullptr){}
    array_view_helper& operator=(array_view_helper const&)=default;

    template<class A>
    explicit operator std::vector<non_const_T, A>() const {
      return { this->begin(), this->end() };
    }
    std::vector<non_const_T> as_vector() const {
      return std::vector<non_const_T>(*this);
    }

    template<std::size_t N>
    array_view_helper( T(&arr)[N] ):range<T*>(arr+0, arr+N) {}
    template<std::size_t N>
    array_view_helper( std::array<T,N>&arr ):range<T*>(arr.data(), arr.data()+N) {}
    template<class A>
    array_view_helper( std::vector<T,A>&vec ):range<T*>(vec.data(), vec.data()+vec.size()) {}
    array_view_helper( T*s, T*f ):range<T*>(s,f) {}
  };
}
// non-const
template<class T>
struct array_view:details::array_view_helper<T> {
  using base = details::array_view_helper<T>;

  // using base::base in C++11 compliant compilers:
  template<std::size_t N>
  array_view( T(&arr)[N] ):base(arr) {}
  template<std::size_t N>
  array_view( std::array<T,N>&arr ):base(arr) {}
  template<class A>
  array_view( std::vector<T,A>&vec ):base(vec) {}
  array_view( T*s, T*f ):base(s,f) {}

  // special methods:
  array_view( array_view const& ) = default;
  array_view() = default;
  array_view& operator=(array_view const&)=default;
};
template<class T>
struct array_view<T const>:details::array_view_helper<const T> {
  using base = details::array_view_helper<const T>;

  // using base::base in C++11 compliant compilers:
  template<std::size_t N>
  array_view( std::array<T const,N>&arr ):base(arr) {}
  array_view( T const*s, T const*f ):base(s,f) {}
  template<std::size_t N>
  array_view( T const(&arr)[N] ):base(arr) {}

  // special methods:
  array_view( array_view const& ) = default;
  array_view() = default;
  array_view& operator=(array_view const&)=default;

  // const T only constructors:
  template<std::size_t N>
  array_view( std::array<T,N> const&arr ):base(arr.data(), arr.data()+N) {}
  template<std::size_t N>
  array_view( std::array<T const,N> const&arr ):base(arr.data(), arr.data()+N) {}
  template<class A>
  array_view( std::vector<T,A> const&vec ):base(vec.data(), vec.data()+vec.size()) {}
  array_view( std::initializer_list<T> il):base(il.begin(), il.end()) {}
};

which is at least a sketch of some view classes. live example

Then expose an array_view<MyClass> as a static member of your class, which is initialized to the array you created in the .cpp file.

range<It> is a range of iterators that acts like a non-owning container. Some tomfoolery is done to block non-constant-time calls to size or [] at the SFINAE level. back() is exposed and simply fails to compile if you call it on invalid iterators.

A make_range(Container) makes range<It> more useful.

array_view<T> is a range<T*> that has a bunch of constructors from contiguous buffer containers, like C-arrays, std::arrays and std::vectors. (actually an exhaustive list).

This is useful because access through an array_view is about as efficient as access to a raw pointer-to-first-element of an array, but we get many of the nice methods that containers have, and it works with range-for loops. In general, if a function takes a std::vector<T> const& v, you can replace it with a function that takes a array_view<T> v and it will be a drop-in replacement. The big exception is operator vector, which is explicit, to avoid accidental allocations.

Yakk - Adam Nevraumont
  • 262,606
  • 27
  • 330
  • 524
  • your suggested code has three errors in the `array_view` class, which I'm not sure how to fix. 1. `array_view():range_view(nullptr, nullptr){}` gives the following errors at `<` character - `Expected '(' or '{'` and `Expected '[' or ','`. 2. `T* data() const { return begin(); }` gives error `Use of undeclared identifier 'begin'` and 3. `return { begin(), end() };` gives two errors `Use of undeclared identifier 'begin'` and `Use of undeclared identifier 'end'` – DarkMatter Oct 20 '14 at 18:04
  • @DarkMatter fixed. Sorry, had a working version, added features, didn't recompile. – Yakk - Adam Nevraumont Oct 20 '14 at 18:07
  • Just tested it and everything seems to work. Have you used this in your own code, or did you just come up with it to answer the question ? Can't say I understand all the template shenanigans, but from my initial testing, this seems to be the best solution at the moment, since it's the only solution that is capable of meeting all the criteria I specified. Kudos to you! – DarkMatter Oct 20 '14 at 19:37
  • @DarkMatter I have written similar systems elsewhere (with similar names even), but I wrote this whole hog from my head. The SFINAE method technique is different this time (I did CRTP base conditional methods last time I wrote `range`). The `range` template mojo is "am I random access, and if so I have a fast `size()` and `[]`". The `array_view` is about supporting both `T` and `const T` types in the same class (probably better to split `array_view` into two specializations, one for `T` and one for `const T`, as it would get rid of that noise). – Yakk - Adam Nevraumont Oct 20 '14 at 19:49
  • **This should be included in the STL!** (you should maybe consider a proposal). I just can't believe the C++ standard doesn't already have a simple solution to what should be a trivial problem (ie. declare `std::array MyClass::arr_pre` in `.hpp` and define with `std::array MyClass::arr_pre = { MyClass{1,2,3}, MyClass{4,5,6} };` in `.cpp`). As you mention though, your solution has the advantage of hiding the type of storage used (although I'm not sure it matters in this case since the storage type can convey that operations that you can use on it - ie. for-range etc.) – DarkMatter Oct 20 '14 at 19:52
  • My previous tests were with Clang/LLVM 3.5svn. Just tested it on Visual Studio 2013 Update 4 RC and it gives compiler error `'size_t range::size(void) const' : could not deduce template argument for ''`. I assume this is a VC++ compiler bug, but is there any workaround to fix it ? – DarkMatter Oct 20 '14 at 20:03
  • I've unmarked this as the answer for the moment, as while it's a great solution, it's not portable to Visual Studio. I know this wasn't an original criteria, but if it doesn't work with the main compiler on one of the major platforms, then it's not really a workable solution. If it can be made compatible with VC++ 2013 Update 4, I'll mark it as answer again. Added VC++ 2013 compatibility as one of the criteria. – DarkMatter Oct 20 '14 at 20:10
  • @DarkMatter Don't care much about the flag, honestly. But I did rewrite it in a way that might work on VS 2013. Also does less template tomfoolery through the magic of CRTP. I don't know if VS2013U4RC supports inheriting constructors the way I did it. – Yakk - Adam Nevraumont Oct 20 '14 at 20:31
  • Thanks for your persistance with this. I tried your latest version and VC++2013U4RC complains about the two lines `using base::base;`, saying `using-declaration causes a multiple declaration of 'base'` – DarkMatter Oct 20 '14 at 21:31
  • @DarkMatter constructors inherited manually, as [msvc lacks support for C++11](http://msdn.microsoft.com/en-us/library/hh567368.aspx) in that area, among others. – Yakk - Adam Nevraumont Oct 20 '14 at 22:09
  • latest version now also works on VC++2013 Update 4 RC. Marked as answer, as this meets all the criteria, and seems like the only solution to the problem currently. Many thanks for your help! – DarkMatter Oct 20 '14 at 22:42
1

I personally like your constexpr static int my_array[] = {MyClass{1, 2, 3}, MyClass{1, 2, 3}}; I don't think you should shy away from that if a C-style array meets your needs.

If you really want to use std::vector though you could use static const std::vector<MyClass*> vec_pre;. So your .cpp file would have this at the top:

namespace{
    MyClass A{1, 2, 3}, B{1, 2, 3}, C{1, 2, 3};
}
const std::vector<MyClass*> MyClass::vec_pre{&A, &B, &C};

EDIT after DarkMatter's comments:

After reading your comments, it looks like there could be some maintainability hazard to my method. It could still be accomplished like this in your .cpp:

namespace{
    MyClass temp[]{MyClass{1, 2, 3}, MyClass{1, 2, 3}, MyClass{1, 2, 3}};
    const MyClass* pTemp[]{&temp[0], &temp[1], &temp[2]};
}
const std::vector<MyClass*> MyClass::vec_pre{begin(pTemp), end{pTemp}};

You could also remove the duplication of entry maintainability problem by creating a macro to do that for you.

Community
  • 1
  • 1
Jonathan Mee
  • 37,899
  • 23
  • 129
  • 288
  • The problem with using `constexpr static int my_array[] = {MyClass{1, 2, 3}, MyClass{1, 2, 3}};` is that it needs to be in the header and so changes to the array require re-compilation of all files that use it. – DarkMatter Oct 20 '14 at 03:29
  • Also, my `constexpr` "work-around" was incorrect (copy/paste error) as the type should be `MyClass` rather than `int`. When corrected to `static MyClass my_array[] = {MyClass{1, 2, 3}, MyClass{1, 2, 3}};` the compiler gives an error `Invalid use of incomplete type MyClass` I'm thinking `std::array` is the way to go with `make_array()`, although I need to check it works with the new C++11 `for-range` syntax – DarkMatter Oct 20 '14 at 03:50
  • Not a fan of the other unnamed method as you need to create new variables for each entry and also add it to the `vec_pre` intializer_list, which isn't great for maintainability. – DarkMatter Oct 20 '14 at 03:54
  • 1
    @DarkMatter Not sure if you get a notification when I edit my answer, but I updated it to respond to you. I also asked this question to see if there was a more direct method for getting pointers into a `std::vector`: http://stackoverflow.com/questions/26466049/const-vector-of-pointers-to-c-style-array-elements – Jonathan Mee Oct 20 '14 at 15:18
  • It just seems like a messy/over-complicated way to what should be a simple efficient process. – DarkMatter Oct 20 '14 at 17:50
  • @DarkMatter I tried to go for as simple as possible on this solution. But I understand what you're saying that a simple definition may lead to increased code complexity down the road. As the author, you are the only one who can define what "simple" is for this problem, so I defer to your judgement. – Jonathan Mee Oct 20 '14 at 18:11
0

Here is a way to set up the vector without copies or moves.

It doesn't use a braced initializer but your opening paragraph suggests that your main concern is avoiding copies and moves; rather than an absolute requirement to use a braced initializer.

 // header
const std::vector<MyClass> &get_vec();

// cpp file
const std::vector<MyClass> &get_vec()
{
    static std::vector<MyClass> x;

    if ( x.empty() )
    {
        x.emplace_back(1,2,3);
        x.emplace_back(4,5,6);
        // etc.
    }

    return x;    
}
M.M
  • 138,810
  • 21
  • 208
  • 365