Initializing pointer to vector of objects inside another object

Question

i want to create a somewhat complex structure of classes in an arduino. The short story is: a Device has Presets, and each Preset has a Patterns. For that i created the following structure (i am only posting the important stuff, if something is missing let me know):

INO file

#include "LedDevice.h"
LedDevice device;
void setup() {
 device = LedDevice(10, 10, 10);
}
void loop() {
 Serial.println(F("Ticking"));
 device.Tick();
 delay(500);
}

LedDevice

#if !defined(_LEDDEVICE_H)
#define _LEDDEVICE_H
#include "Preset.h"
#include<Arduino.h>
class LedDevice {
public:
 Preset* Presets[];
 LedDevice(){}
 LedDevice(int numPixels, int numPresets, int numPatterns)
 {
 *Presets = (Preset*)malloc(sizeof(Preset) * numPresets);
 for (int p = 0; p < numPresets; p++)
 {
 Presets[p] = new Preset("test", numPixels, numPatterns);
 }
 }
 void Tick()
 {
 Serial.print(F("Preset ")); Serial.println(Presets[0]->Name);
 Presets[0]->Tick();
 }
};
#endif //_LEDDEVICE_H

Preset

#if !defined(_PRESET_H)
#define _PRESET_H
#include "Pattern.h"
class Preset {
public:
 char *Name;
 int NumPatterns;
 Pattern* Patterns[];
 Preset(char* name, int numPixels, int numPatterns) : NumPatterns(numPatterns)
 {
 Name = (char *)malloc(strlen(name) + 1);
 strcpy(Name, name);
 *Patterns = (Pattern*)malloc(sizeof(Pattern) * NumPatterns);
 for (int p = 0; p < NumPatterns; p++)
 {
 Patterns[p] = new Pattern(numPixels);
 }
 }
 void Tick()
 {
 for (int p = 0; p < NumPatterns; p++)
 {
 Patterns[p]->Tick();
 }
 }
};
#endif

Pattern

#if !defined(_PATTERN_H)
#define _PATTERN_H
class Pattern {
public:
 Pattern(int numPixels){}
 void Tick(){}
};
#endif

The code compiles but when i run the full version (this is a simplified version of the code) i get some strange behavior, so i would like to know if i am doing the initialization of the Presets and Patterns correctly.

Also when i try to print the Preset name in the LedDevice Tick function i get strange characters, so either i am not initializing the string correctly or the pointers to the Presets are wrong.

Can you tell if anything is not being done correctly in the constructors?

• James Waldby - jwpat7: You may have provided enough info – I don't know – but I think it would be helpful if you edit your question to include a Minimal, Complete, and Verifiable example of code, as well as the snippets. Otherwise, anyone who wishes to test an answer before posting it has to do the busy work of putting the snippets together.
• James Waldby - jwpat7: My concept of what new does is: allocate memory for an object and construct the object. Doesn't *Presets = (Preset*)malloc(sizeof(Preset) * NumPresets); allocate memory for an array of Presets and for ... Presets[p] = new Preset(...) allocate the same amount again? (This would be a problem, not the problem.)
• Michel Keijzers: I'm new to Arduino, but isn't it possible to use a 2 dimensional array (of Pattern)? Or does Device has more properties than only a list an array of patterns?
• Luis Ferreira: @jwpat7 I added a complete version of the code. If i remove the New keyword from the objects initialization it complains that "cannot convert 'Preset' to 'Preset*' in assignment", so i tought it was still necessary despite the malloc.
• Luis Ferreira: @MichelKeijzers each class, LedDevice, Pattern and Preset has other attributes and functions, therefore i need a class for each, even a structure won't do. I'm already having issues initializing 1 dimensional array, if i go for 2 i guess it will be even worse

Answer 1

Three things that need to be different:

Pattern* Patterns[];

should be

Pattern** Patterns;

It's a pointer to some pointers to patterns, not a pointer to an array (which is basically a pointer itself so same basic idea but an array implies a known size).

and

*Patterns = (Pattern*)malloc(sizeof(Pattern) * NumPatterns);

should be

Patterns = (Pattern*)malloc(sizeof(Pattern*) * NumPatterns);

*Patterns is the thing pointed to by Patterns which at this point is undefined. You want to set Patterns to the allocated address. And we only need to allocate space for the pointers not the actual objects.

Your for loops then become

for (int p = 0; p < NumPatterns; p++)
{
 (Patterns+p) = new Pattern(numPixels);
}

You then have the same issues with Presets.

And finally

LedDevice device;
void setup() {
 device = LedDevice(10, 10, 10);
}

should be

LedDevice device(10, 10, 10);
void setup() { 
}

• Luis Ferreira: Thank you for the input, some of those errors i was aware, and others i didn't know, but i can understand the logic now.
• Andrew: Sorry, just realised that the origional post was wasting too much memory, As Nick pointed out the new command is allocating the space for the actual objects. All you need to malloc is enough space for the pointers to those objects rather than enough space for the entire objects.

Answer 2

This is wrong:

Device device;
void setup() 
{
 device = Device();
}

I don't know what your intentions are, but the first line instantiates device. You don't need to do anything in setup here. It sort-of looks like you are trying to call the constructor but that is called when the class instance is created (in the first line).

---

Preset* Presets[];

What's this? A pointer to an array of zero length?

---

 *Presets = (Preset*)malloc(sizeof(Preset) * NumPresets);

Now you are allocating memory for a pointer to the array to be big enough to hold the contents of the array.

---

for (int p = 0; p < NumPresets; p++)
{
 Presets[p] = new Preset("test", NumPixels, NumPatterns);
} 

Now you are allocating memory for the contents of the array again.

---

There is too much dereferencing going on here. Please post a Minimal, Complete, and Verifiable example. For example, make the classes in question do not much. Then make code we can actually work on.

---

I realize the first call is to the default constructor, but in the setup i do the real construction ...

Well, you can't call the constructor twice. What is commonly done is to do nothing (or almost nothing) in the constructor, and then make a begin function which you call in setup. This function can then do whatever is required at the proper time.

• Luis Ferreira: I just updated the code to a Complete version. Here are some answers: - i declare Device, now named LedDevice outside of the setup because i want to use it in the Loop cycle as well. Is there another way to do it? I realize the first call is to the default constructor, but in the setup i do the real construction - I would like to be able to initialize the Preset and Pattern vector during run-time, so i am creating a pointer to a vector that i don't know the size yet. At least that's what i tought, is this wrong? - so i dont need to use both malloc and new? what is the correct way?
• Nick Gammon: See amended answer.
• Nick Gammon: malloc allocates memory. new also allocates memory but also calls the constructor. You should use new when allocating memory for classes.