initialize array in constructor

Question

I want to have a class which has a member array. The size of the array should be given when I initialize the object. I just found a way with pointers to do this. I think it is working correctly, but can you maybe tell me if this is the best way to do it or if there are any things which do not work which I haven't recognized yet?

#include <iostream>
using namespace std;

class Surface {
  private:
    float dx;
    int N;
    float* mesh_points;

  public:
    Surface(float, int);
    ~Surface();
    void set_dx (float);
    float get_dx();
};

Surface::Surface(float dx,int N){
  this->dx = dx;
  this ->N = N;
  mesh_points = new float[N];
}


void Surface::set_dx (float dx) {
  this->dx = dx;
}


float Surface::get_dx (void) {
  return dx;
}

Surface::~Surface(){
  delete[] mesh_points;
}

int main () {
  Surface s(1.2,10);
  s.set_dx (3.3);
  cout << "dx: "<< s.get_dx() <<endl;

  float mesh_points[3];
  return 0;
}

Best way to do is start using std::vector. It give you a facility to use dynamic size array. Also it does all the optimized memory mgmt while resizing itself. — Rizwan, Sep 17 '18 at 10:08
[`std::vector`](https://en.cppreference.com/w/cpp/container/vector) exists entirely for this purpose - a dynamically sized array that does all the memory management for you (as it's notoriously fiddly to get right). — BoBTFish, Sep 17 '18 at 10:08
In addition to what was stated: your class lacks copy-constructor, and copy-assignment operator. If your class instances are ever assigned to each other, or copied, you would invoke undefined behavior due to double-freeing the same memory. — Algirdas Preidžius, Sep 17 '18 at 10:12
Your code fails to follow the [rule of three](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) so, for instance, a simple program like `int main() { Surface s(1.2,10); Surface t(s); }` will fail. Like others have said the **easy** way to do this is to use `std::vector`. — john, Sep 17 '18 at 10:12
Also, you may wish to reconsider your use of what are often considered bad practices: [`using namespace std;`](http://stackoverflow.com/q/1452721/1171191) and [`endl`](http://chris-sharpe.blogspot.co.uk/2016/02/why-you-shouldnt-use-stdendl.html) ([another for `endl`](https://www.youtube.com/watch?v=6WeEMlmrfOI)) (those are links to explanations). — BoBTFish, Sep 17 '18 at 10:14
As I understood vector is preferable if you want to change size or remove elements from your vector. But this I don't want to do. I just want to choose the size at the beginning and then fix it. How much more memory does the usage of std::vector require? In the end I will have huge vectors... — Jan SE, Sep 17 '18 at 10:22
@JanSE A few bytes. The only additional information the `vector` stores over the raw dynamic memory is the occupied "size" (which is different from "capacity"). This is virtually unimportant if you have "huge" vectors; (it'd be more important if you had *a lot of small vectors*). — Bartek Banachewicz, Sep 17 '18 at 10:25
`vector` also implements copy/move constructors/operators and your overall class will get more readable (you could skip `N` and the destructor) — dan, Sep 17 '18 at 10:28
Can you do `C++1x`? If yes, then you can use std::array and its clean initialization syntax. — bobah, Sep 17 '18 at 10:33
@bobah `std::array` requires compile-time size, which isn't available here, so it's not a good suggestion. — Bartek Banachewicz, Sep 17 '18 at 10:34
@BartekBanachewicz - I thought I'd still propose it for completeness, along with [`std::unique_ptr`](https://stackoverflow.com/a/16711846/267482) — bobah, Sep 17 '18 at 10:38

Bartek Banachewicz · Accepted Answer · 2018-09-17T13:39:57.490

can you maybe tell me if this is the best way to do it or if there are any things which do not work which I haven't recognized yet?

That'd be my take basing on existing best practices:

class Surface {
private:
    std::vector<float> mesh_points;

public:
    float dx;

    Surface(float dx, std::size_t n);
};

Surface::Surface(float dx, std::size_t n)
  : dx(dx)
  , mesh_points(n)
{
}

In short, the changes made:

Got rid of manual memory management, which implies dtor as well.
Added names for parameters in declarations (really important for usability/IDEs, don't remove them!)
Got rid of superfluous accessors and made dx public.
Used ctor init lists, making the body obsolete.
Got rid of using namespace std; in lieu of explicit std:: prefix.
Changed n type to std::size_t (see comment).

Please note that the current interface doesn't allow any access to mesh_points.

For consistency , the type of the second argument of the constructor would be better specified as the same type used by `std::vector` for sizes. Formally, that is `std::vector::size_type`. Practically it is often `std::size_t`. It is never a `signed` type, such as `int`. — Peter, Sep 17 '18 at 12:11

score 0 · Answer 2 · answered Sep 17 '18 at 10:41

Here's an alternative suggestion that allows you to keep your current implementation but is a lot safer.

class Surface {
  private:
    float dx;
    int N;
    float* mesh_points;

  public:
    Surface(float, int);
    ~Surface();
    void set_dx (float);
    float get_dx();
    Surface(const Surface&) = delete;            // new
    Surface& operator=(const Surface&) = delete; // new
};

By deleting the implementation of the copy constructor and copy assignment operator you prevent your Surface objects from being copied (which would likely crash your program anyway). Any attempt to copy Surface objects will now result in a compile time error.

Only a suggestion, my first choice would always be to use std::vector.

initialize array in constructor

2 Answers2