5

Before I start with my question, let me say that I know that I can solve my problem easily using the standard library or gsl library (e.g. std::Vector). But I'm trying to learn C++ and dynamic memory allocation, so I would like to know if there is a way to solve it without using a Vector or anything similar.

I have a Layer class that contains a number of neurons determined when the object is created (through dynamic memory allocation):

class Layer {
private:
    unsigned int _size;
    const Neuron* _neurons;

public:
    Layer(unsigned int size);
    ~Layer();

    const Neuron* getNeuron(unsigned int index) const;
};

Layer::Layer(unsigned int size) {
    _size = size;
    _neurons = new Neuron[size];
}

const Neuron* Layer::getNeuron(unsigned int index) const {
    Expects(index < _size);

    return &_neurons[index];
}

Layer::~Layer() {
    delete[] _neurons;
}

Now the challenge is in the Network class: I need to create a Network of n layers, each containing a different number of neurons (passed as an array of length n):

class Network {
private:
    unsigned int _nb_layers;
    const Layer* _layers;

public:
    Network(unsigned int nb_layers, unsigned int* nb_neurons);
    ~Network();

    const Layer* getLayer(unsigned int index) const;
};

Network::Network(unsigned int nb_layers, unsigned int *nb_neurons) {
    _nb_layers = nb_layers;
    _layers = new Layer[nb_layers];

    for(int i = 0; i < nb_layers; i++) {
        _layers[i] = new Layer(nb_neurons[i]);
    }
}

const Layer* Network::getLayer(unsigned int index) const {
    Expects(index < _nb_layers);

    return &_layers[index];
}

Network::~Network() {
    delete _layers;
}

The problem is that the Network constructor fails, because _layers = new Layer[nb_layers] tries to call the constructor with no parameters, which fails. Also, _layers[i] = new Layer(nb_neurons[i]) fails because "No viable overloaded '='", which I do not understand. How can I fix this using dynamic memory allocation instead of std::Vector?

Finally, is the way I implemented dynamic memory allocation correct and without any memory leak? I'm wondering what happens to my unsigned int *nb_neurons in the Network constructor since the values are being used but I'm never deleting it. (As a background, I'm been coding in Java, Python and PHP for years)

Thank you very much!

Drade
  • 157
  • 10
  • 1
    If you come from Java environment, you'd better forget everything you learned there and treat C++ as a completely new language. For one thing, `new` is considered bad practice in modern C++ and you definitely don't want to create every single object with `new`. – Yksisarvinen Aug 07 '19 at 21:58
  • For array of objects without default constructor: [Object array initialization without default constructor](https://stackoverflow.com/questions/4754763/object-array-initialization-without-default-constructor), for possible memory leaks: [What is The Rule of Three?](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) (not really a leak, but an issue nevertheless) – Yksisarvinen Aug 07 '19 at 21:59
  • 1
    vectors *are* dynamic memory allocation. – M.M Aug 07 '19 at 22:01
  • Yes I have learned that 'new' is not recommended by c++ core guidelines, I'm just trying to use it to learn how it works before moving on to the other ways of doing it. – Drade Aug 07 '19 at 22:01
  • I posted an answer showing how to use a contiguous array of Layers using placement-new. It's much closer to what you wanted in your original question, and you don't have the extra layer of indirection that comes from using an array of pointers (meaning it's more memory-efficent too). – Alecto Irene Perez Aug 07 '19 at 22:20

3 Answers3

4

I need to create a Network of n layers, each containing a different number of neurons (passed as an array of length n):

You won't be able to do that with a fixed array of Layer objects. Although there is a syntax available for the new[] operator to initialize all of the elements of the allocated array with the same constructor value, there is no syntax for initializing them with different values, at least not when the values are coming from another array.

To do what you are asking for, you will have to create an array of Layer* pointers (or better, an array of std::unique_ptr<Layer> objects in C++11 and later), and then dynamically create each Layer object with a different constructor value as needed. You were close in doing that, but you are just missing an extra layer of indirection in your array declaration:

class Network {
private:
    unsigned int _nb_layers;
    Layer** _layers; // <-- DYNAMIC ARRAY OF POINTERS, NOT OBJECTS!

public:
    Network(unsigned int nb_layers, unsigned int* nb_neurons);
    ~Network();

    const Layer* getLayer(unsigned int index) const;
};

Network::Network(unsigned int nb_layers, unsigned int *nb_neurons) {
    _nb_layers = nb_layers;
    _layers = new Layer*[nb_layers]; // <-- ALLOCATE ARRAY OF POINTERS FIRST

    for(int i = 0; i < nb_layers; i++) {
        _layers[i] = new Layer(nb_neurons[i]); // <-- THEN MAKE EACH POINTER POINT TO AN OBJECT
    }
}

const Layer* Network::getLayer(unsigned int index) const {
    Expects(index < _nb_layers);

    return _layers[index];
}

Network::~Network() {
    for (int i = 0; i < _nb_layers; ++i)
        delete _layers[i]; // <-- NEED TO FREE EACH OBJECT FIRST
    delete[] _layers; // <-- THEN FREE THE ARRAY ITSELF
}

In C++11 and later, you can do this instead:

class Network {
private:
    unsigned int _nb_layers;
    std::unique_ptr<std::unique_ptr<Layer>[]> _layers;

public:
    Network(unsigned int nb_layers, unsigned int* nb_neurons);
    // ~Network(); // <-- NOT NEEDED!

    const Layer* getLayer(unsigned int index) const;
};

Network::Network(unsigned int nb_layers, unsigned int *nb_neurons) {
    _nb_layers = nb_layers;
    _layers.reset( new std::unique_ptr<Layer>[nb_layers] );

    for(int i = 0; i < nb_layers; i++) {
        _layers[i].reset( new Layer(nb_neurons[i]) );
        // or, in C++14 and later:
        // _layers[i] = std::make_unique<Layer>(nb_neurons[i]);
    }
}

const Layer* Network::getLayer(unsigned int index) const {
    Expects(index < _nb_layers);

    return _layers[index].get();
}

/*
Network::~Network()
{
    // NOTHING TO DO HERE!
}
*/

And BTW, both of your classes are violating the Rule of 3/5/0. Neither of the classes is implementing a copy constructor or a copy assignment operator in order to make copies of their respective arrays from one class instance to another. Or, in the case of C++11 and later, a move constructor and a move assignment operator to move their respective arrays from one class instance to another.

Remy Lebeau
  • 555,201
  • 31
  • 458
  • 770
  • Thank you, this is very clear (I'll look up the Rule of 3/5/0, this seems pretty important). I have a question about std::unique_ptr, I understand it is useful as it deletes the object if the pointer becomes inaccessible, but if you implement pointers correctly and delete everything then are they useful in any way? – Drade Aug 07 '19 at 22:11
  • @aurelien Smart pointers allow you to write safer memory management code. Any time you can replace `delete`/`delete[]` with `std::unique_ptr` (or `std::shared_ptr`), you should. Modern C++ code should avoid calling `new`/`new[]` directly whenever possible. In this case, a `std::vector` would be a better choice, but `std::unique_ptr` offers a compromise so you can still use `new`/`new[]` manually without having to use `delete`/`delete[]` manually. – Remy Lebeau Aug 07 '19 at 22:13
0

The line

_layers[i] = new Layer(nb_neurons[i]);

suffers from two problems.

  1. There is a type mismatch. _layers[i] is an object while the RHS is a pointer.
  2. _layers[i] is a const object. Hence, you can't assign anything to it after it is initialized.

You can resolve the above problem as well as the default constructor problem using

std::vector<Layer*> _layers;

For better and easier memory management, use a vector of smart pointers.

std::vector<std::unique_ptr<Layer>> _layers;

or

std::vector<std::shared_ptr<Layer>> _layers;
R Sahu
  • 204,454
  • 14
  • 159
  • 270
0

You can do this, but you'll have to use placement new. Let's look at how this could be done.

First, we're going to create a class that represents a block of memory. This will be used in conjunction with placement new to create layers. It doesn't need any functions, because it's just used for the purposes of allocating memory of the right size and alignment.

template<size_t count, size_t alignment>
struct alignas(alignment) memory {
    std::array<unsigned char, count> bytes;
};

The two parameters given here allow us to specify the size and alignment of the block of memory. In our case, we want blocks of memory with the same size and alignment of a Layer:

using layer_memory = memory<sizeof(Layer), alignof(Layer)>; 

We can use this to create a class representing an array of layers. I split it up into four parts: - The data members (a pointer to the memory, and a variable storing the number of layers - The member functions (used to access individual layers) - The constructors (used to make the LayerArray from a list of sizes - The destructor (used to delete the block of memory we allocated)

class LayerArray {

    //* PART ONE: DATA MEMBERS *//
   private:
    layer_memory* mem;
    size_t count;

    //* PART TWO: MEMBER FUNCTIONS *//
   public:
    // Get a pointer to the memory as an array of Layers
    Layer* data() {
        return reinterpret_cast<Layer*>(mem); 
    }
    Layer const* data() const {
        return reinterpret_cast<Layer const*>(mem); 
    }
    // Dereference the i-th block of memory as a Layer
    Layer& operator[](size_t i) {
        return data()[i]; 
    }
    Layer const& operator[](size_t i) const {
        return data()[i]; 
    }

    //* PART THREE: CONSTRUCTORS *//

    // Convenience constructor from initializer list
    LayerArray(std::initializer_list<unsigned> counts)
      : LayerArray(counts.begin(), counts.size()) {}

    // Move constructor
    LayerArray(LayerArray&& other)
      : mem(other.mem)
      , count(other.count)
    {
       other.mem = nullptr;
       other.count = 0;
    }

    // Constructor that uses an array of layer sizes
    LayerArray(const unsigned int* layer_size_counts, size_t count)
      : mem(new layer_memory[count])
      , count(count) 
    {
        for(size_t i = 0; i < count; i++) {
            auto layer_size = layer_size_counts[i]; 

            // Create the new layer in the i-th block of memory
            new (mem + i) Layer(layer_size); 
        }
    }

    //* PART FOUR: DESTRUCTOR *//
    ~LayerArray() {
        for(size_t i = 0; i < count; i++) {
            Layer& layer = data()[i]; // Get the layer
            layer.~Layer(); // Manually call the destructor for the Layer
        }
        delete[] mem; 
    }
};

This allows us to create a contiguous array of layers, and instantiate them with different sizes:

int main() {
    // Create an array of layers with the specified sizes
    LayerArray layers { 50, 50, 50, 10, 3 };

    // Do other stuff...
}
Alecto Irene Perez
  • 10,321
  • 23
  • 46