Can't & proper way to declare an array of arrays of int in class member c++11

Question

i'm making a simple map with defined size as parameter, i would like it to be stored in my private member variable.

I will show a simple exemple :

class A {
   public:
   A (const int size) {
     map_size_ = size;
     //or | both will not compile
     int map[size][size];


   }
   private:
    int map_size_;
    int map_[map_size_][map_size_];
}

I know the map_ won't compile, but i wonder how to declare it properly with a const int from the constructor parameter and without pointers.

Eljay · Answer 1 · 2017-12-01T23:58:40.897

0

This should work:

class A {
public:
  A(int const size) {
    _map_size = size;
    _map = new int[_map_size * _map_size];
   }
private:
  int _map_size;
  int* _map;
};

UPDATE - apparently people want a more comprehensive solution than the OP asked about.

class A {
public:
  A(int const size)
    : _map_size(size)
    , _map(new int[_map_size * _map_size]) {
   }
  A(A const&) = delete;
  ~A() {
    delete[] _map;
  }
  A& operator=(A const&) = delete;
  int at(int x, int y) const {
    if (x < 0 || x >= _map_size || y < 0 || y >= _map_size)
      throw "That coordinate is in the giggleweeds";
    return _map[y * _map_size + x];
  }
  int set_at(int x, int y, int value) {
    if (x < 0 || x >= _map_size || y < 0 || y >= _map_size)
      throw "That coordinate is in the giggleweeds";
    _map[y * _map_size + x] = value;
  }
private:
  int _map_size;
  int* _map;
};

edited Dec 01 '17 at 23:58

answered Dec 01 '17 at 23:52

Eljay

4,648
3
16
27

Throwing strings? You gotta be kidding me! And why is that `set_at` needed? Just standard `int &at` will do! – Dec 02 '17 at 00:01
@Ivan • The user requirements are rather sparse on that point. – Eljay Dec 02 '17 at 00:02
1

A bit more versatile than `at` and `setAt`: `int & operator()(size_t x, size_t y) { if (x < _map_size && y < _map_size) { return _map[y * _map_size + x] ;} throw std::out_of_range("That coordinate is in the giggleweeds"); }` – user4581301 Dec 02 '17 at 00:18
1

@user4581301 • if following standard library conventions, the at() ones should be checked, and the (x,y) functor should be unchecked. – Eljay Dec 02 '17 at 01:31
I'll agree with that. And there probably should be `const` versions of both. – user4581301 Dec 02 '17 at 05:21

score 0 · Accepted Answer · answered Dec 01 '17 at 23:55

0

You should use std::vector

#include <vector>

class A {
public:
    explicit A (const int size) {
         map_.resize(size);
         for (auto &v : map_) v.resize(size);
    }

private:
    std::vector<std::vector<int>> map_;
};

This might not be the most optimal way from performance point of view, but it is the simplest one. There is no way around that. int[][] declarations do not work with non-compile time values in C++.

Tips:

Use explicit with single argument constructors.
You should use std::size_t instead of int for sizes.

answered Dec 01 '17 at 23:55

I like the use of the for-each, very clean. – Eljay Dec 02 '17 at 00:01
1

It is not necessary to use `resize()` in the constructor. Simply construct the vector directly with the required size., rather than relying on default construction followed by resizing. – Peter Dec 02 '17 at 00:10
@Eljay very clean, but completely unnecessary. The `vector` will do all this free of charge for you if you take advantage of the member initializer list. – user4581301 Dec 02 '17 at 00:37
@user4581301 No it won't. If you are suggesting to use `map_(size, std::vector(size))`, then I must say this is a _horrible_ idea. It won'be free of charge as this vector will be copied and copying data is slower than memsetting it. Plus `std::vector` is already horrendously slow at large allocations due to it being unable to avoid memory initialization after allocation zero pages from the kernel. – Dec 02 '17 at 00:41
Never had a problem with it, but probably because I wouldn't allocate a `vector` of `vector`s if I needed speed. I'd do what @Eljay did, but with a `vector`. If I couldn't tolerate the cost of the zero initialization, I'd ignore the asker's no pointers request and do almost exactly what Eljay proposed. – user4581301 Dec 02 '17 at 00:55
@user4581301 Has nothing to do with `vector` of `vector`s, plain `std::vector` is horribly slow at large allocations. OP didn't ask for anything specifically, using `vector` gives the shortest, simplest code that is able to take advantage of automatic destructor, copy constructor, copy assignment, move constructor and move assignment generation. Anyway, proposed change is cosmetic only, aside from giving worse performance. – Dec 02 '17 at 01:00
Why? Why would `vector` be any slower than any other dynamic allocation? What consideration am I missing here? Do we just have a greatly different understanding of horrible? – user4581301 Dec 02 '17 at 05:20
@user4581301 `vector` initializes every element in allocated memory chunk, for POD types that means zeroing it out. But for large allocations (usually around 128k) allocators request memory directly from the kernel. This memory will be filled with zeroes already for security reasons. `vector` will be wasting time zeroing out zero pages. And that is why `vector` should not be used for large allocations. Read https://stackoverflow.com/questions/2688466/why-mallocmemset-is-slower-than-calloc – Dec 02 '17 at 10:37
Alright. We just have a greatly different understanding of horrible. In my opinion you are overestimating the impact of initialization or find yourself in a position where you are initializing vectors very frequently. – user4581301 Dec 02 '17 at 16:26

Can't & proper way to declare an array of arrays of int in class member c++11

2 Answers2