Array as private property of class

So my problem is with the array ledContainer. Thing is that I want it to be a private property. But when running verify I get:

LEDDimmer:25:22: error: flexible array member 'LEDDimmer::ledContainer' not at end of 'class LEDDimmer'
LED ledContainer;
^
/home/alexandru/Arduino/Projects/led_potentiamotor/LEDDimmer.ino: In member function 'void LEDDimmer::addLED(int, int)':
/home/alexandru/Arduino/Projects/led_potentiamotor/LEDDimmer.ino:12:42: warning: array must be initialized with a brace-enclosed initializer [-fpermissive]
LED objectCounter[objectCounter] = newLED;
^~~~~~
LEDDimmer:13:24: error: incompatible types in assignment of 'int' to 'LED [((LEDDimmer*)this)->LEDDimmer::objectCounter]'
objectCounter += 1;
^
exit status 1
redefinition of 'class LED'

Although as pert this the declaration is correct. Any suggestions ?

The code:

class LED{
  public:
    LED (int _pin, int _pos){
      pin = _pin;
      pos = _pos;
      lowerBound = 1023 * pos;
      if (lowerBound){
        lowerBound += 1;
      }
      upperBound = 1023 * (pos + 1);

      digitalWrite(pin, LOW);
    }

    int getPin(){
      return pin;  
    }

    void enable(){
      digitalWrite(pin, HIGH);
    }

    void disable(){
      digitalWrite(pin, LOW);
    }

    void updateBrightness(int rawBrightness){
      int convertedBrightness = map(rawBrightness, lowerBound, upperBound, 0, 255);
      convertedBrightness = convertedBrightness > 255 ? 255 : convertedBrightness;

      if (rawBrightness >= lowerBound){
          _updateBrightness(convertedBrightness);
      } else { 
        disable();
      }
    }

  private:
    int pin;
    int pos;
    int lowerBound;
    int upperBound;
    void _updateBrightness(int brightnessIntensity){
      analogWrite(pin, brightnessIntensity);
    }
};
 #include "LED.h"

class LEDDimmer {
  public:
    LEDDimmer(int _potPin){
      objectCounter = 0;
      potPin = _potPin;
    }

    void addLED(int pin, int orderedPos){
      LED newLED(pin, orderedPos);
      LED objectCounter[objectCounter] = newLED;
      objectCounter += 1;
    }

    void updateLED(){
      int brightness = map(analogRead(potPin), 0, 1023, 0, 5115);
      for (int i = 0; i <= objectCounter; i++){
        ledContainer[i].updateBrightness(brightness);
        delay(10);
      }
    }

  private:
    LED ledContainer[];
    int potPin;
    int objectCounter;
};

You need a size

I had it before like this:

LED ledContainer[5];

But still had the same issue

Actually Do you need to hold the instances in there or do you actually want pointers to instances ?

This here don't look right to me, especially given what you seem to want to do with that array of Led objects.

LED doesn't have a default constructor you can use for your array. This compiles:

  private:
    LED ledContainer[5]={LED(4,0),LED(5,1),LED(6,2),LED(7,3),LED(8,4)};
    int potPin;

Except for the issue that Koraks pointed out.

What do you mean that LED does not have a constructor ?

LED (int _pin, int _pos){
      pin = _pin;
      pos = _pos;
      lowerBound = 1023 * pos;
      if (lowerBound){
        lowerBound += 1;
      }
      upperBound = 1023 * (pos + 1);

      digitalWrite(pin, LOW);
    }

This is the constructor, and is the default AFAIK, how come you say it's not?

Haven't really thought about it. I guess pointers are enough.

It's not the default constructor: Default constructors - cppreference.com

A default constructor is a constructor which can be called with no arguments (either defined with an empty parameter list, or with default arguments provided for every parameter).

If you only store the pointers, who owns the LED objects that the pointers point to?

I'm probably doing something wrong here, problem is that I'm not sure what.

My main point is that the user imports the library, and uses the addLED to add new LED to an array of LED.

The led dimmer class.

Well then it shouldn't store pointers to the LEDs, it should store the LEDs themselves.

Either use dynamic allocation (e.g. std::vector - cppreference.com if you're using a platform that supports it), or make the size of the number of LEDs a template parameter of the LEDDimmer class.

If the LED class doesn't have a default constructor, you cannot store an array of LEDs directly. In that case, you'll have to use an array of storage for LED, and then construct the LEDs in-place using the placement new operator. You can find an example here: std::aligned_storage - cppreference.com

1 Like

You could add an additional parameter to LedDimmer, this parameter defines the number of objects to be stored.
Then the constructor of LeddDimmer could allocate an array of LED objects to store the data.
Then you can fill in the members of the LED objects in the array with an additional member function. This is in line with what the posts above indicate.
You can define a default constructor by adding default values for the function parameters.
Below some code as clarification for the principles explained.

class LED {
  public:
    LED (int _pin=-1, int _pos=-1) // default constructor as parameters have default values
    {
      pin = _pin;
      pos = _pos;
      lowerBound = 1023 * pos;
      if (lowerBound)
      {
        lowerBound += 1;
      }
      upperBound = 1023 * (pos + 1);
      digitalWrite(pin, LOW);
    }

    void setPinPos(int _pin, int _pos)
    {
      pin = _pin;
      pos = _pos;
    }
    
    int getPin() 
    {
      return pin;
    }

    void enable() {
      digitalWrite(pin, HIGH);
    }

    void disable() {
      digitalWrite(pin, LOW);
    }

    void updateBrightness(int rawBrightness) {
      int convertedBrightness = map(rawBrightness, lowerBound, upperBound, 0, 255);
      convertedBrightness = convertedBrightness > 255 ? 255 : convertedBrightness;

      if (rawBrightness >= lowerBound) {
        _updateBrightness(convertedBrightness);
      } else {
        disable();
      }
    }

  private:
    int pin;
    int pos;
    int lowerBound;
    int upperBound;
    void _updateBrightness(int brightnessIntensity) {
      analogWrite(pin, brightnessIntensity);
    }
};
#include "LED.h"
class LEDDimmer
{
  public:
    LEDDimmer(int _potPin, int _objectCounterMax)
    {
      objectCounter = 0;
      objectCounterMax = _objectCounterMax;
      // dynamic allocation of array with led objects, size is _objectCounterMax;
      ledContainer = new LED[objectCounterMax];
      potPin = _potPin;
    }
    
    ~LEDDimmer()
    {
      // cleanup of objects:
      delete [] ledContainer;  
    }

    void addLED(int pin, int orderedPos)
    {
      if (objectCounter < objectCounterMax)
        ledContainer[objectCounter++].setPinPos(pin, orderedPos);
    }

    void updateLED() 
    {
      int brightness = map(analogRead(potPin), 0, 1023, 0, 5115);
      for (int i = 0; i <= objectCounter; i++) {
        ledContainer[i].updateBrightness(brightness);
        delay(10);
      }
    }

  private:
    LED *ledContainer;
    int potPin;
    int objectCounter;
    int objectCounterMax;
};

#include "LedDimmer.h"

LEDDimmer Ldd(/*potpin*/1,/*size*/10);

void setup() 
{
  Ldd.addLED(0,10);
  Ldd.addLED(1,20);
  Ldd.addLED(2,30);
  Ldd.addLED(3,40);
}

void loop() 
{
  Ldd.updateLED();
}
1 Like

Interesting, that was exactly what I was looking for. For the most part the code is straightforward, although I have a couple of questions (feel free to answer or direct me to documentation):

  1. LED *ledContainer; what does the * do there ?
  2. ~LEDDimmer() what does the ~ do there ?
  3. Why the need of this ?
  {
      // cleanup of objects:
      delete ledContainer;  
    }
  1. So this acts as if the ledContainer already has initialized objects because LED.h has a default constructor, correct ? ledContainer[objectCounter++].setPinPos(pin, orderedPos);

Really interesting this code sample. I've never written in lower-level languages (my experience is mostly with higher-level ie php, js, python).

it tells the compiler that ledContainer is a pointer to at least one LED instance (actually to an array as dynamically allocated in the constructor ledContainer = new LED[objectCounterMax];) holding the LED instances.)

that's the destructor. because you allocated dynamically the ledContainer array (and created the instances) you need to clean that up to free the memory when the instance is freed.

(EDITED - after I actually read the code :slight_smile: )

1 Like

@tinkerer23
In the old days, there was no memory management with garbage collectors built into the language etc... So you the programmers had to take care of the management themselves. Today, this approach still has it advantages, but that is a subject that would take us too far.
Now, in order to manage references to dynamically (by the program) allocated memory, pointers were invented. (i.e. pointing to a memory that was allocated (or someting else));

int * p; means: p is a pointer to a location that holds the content of an int;
so once you do
p = new int; p will contain the address of a location that can hold an int;
in the same sense;
delete p; will release the memory location for use elsewhere;

in the same sense;
p = new LED[count]; will allocate an space of memory that has enough bytes to contain an array of LED objects; (i.e. the variables as declared in the objects)

for more info: see dr. Google.

~Led() is a destructor,
this function is called when the object goes out of scope or is deleted;

You might want to find a good book about C++ to manage all of this properly, it is worth it.

1 Like

after reading the code...

Actually I think there is a bug in @JOHI code, as ledContainer is just a pointer to a LED instance (LED *) created with this call ledContainer = new LED[objectCounterMax]; when you call delete ledContainer; you will only delete the first element, leading to a memory leak.

You need to iterate on the array and free each LED pointer that was allocated in the destructor.

Changed delete to delete [], I think this corrects the bug?

yes that will do.

@tinkerer23

here is an example (open the serial monitor at 115200 bauds) for you to understand what's at play.

class A {
  public:
    int value = 1000; // default value
    A()  {Serial.println("\t\tA constructor");}                                             // constructor
    ~A() {Serial.print("\t\tA destructor, value being deleted: "); Serial.println(value);}  // destructor
};

class B {
  public:
    B(size_t maxSize) {                                                                     // constructor
      Serial.print("\tB constructor for : "); Serial.println(maxSize);
      myArrayOf_A = new A[maxSize]; // create maxSize instances of A in an array (we ignore memory allocation issues)
      for (size_t i = 0; i < maxSize; i++)  myArrayOf_A[i].value = i;
    }
    ~B() {                                                                                  // destructor
      Serial.println("\t------- B destructor ------- ");
      delete[] myArrayOf_A;
    }
  private:
    A *myArrayOf_A;
};

void setup() {
  Serial.begin(115200); Serial.println();
}

void loop() {
  static uint32_t lastTime = -100000;
  if (millis() - lastTime > 1000) {
    Serial.print("\n\n------- START IF #"); Serial.print(millis()/1000); Serial.println(" ------- ");
    B b(3); // local variable in the if statement, will be constructued upon entering hte block
    lastTime = millis();
    Serial.println("------- END IF ------- ");
    // b local variable will be freed up upon exit of the block, so calling B destructor automatically
  }
}

In the loop we keep allocating and deallocating an instance of B holding 3 sub-instances of A.
Since the allocation is local to the if () {} statement block, the b instance is allocated and then freed up. The Serial prints show what's going on.

Serial Monitor (@ 115200 bauds) will show

------- START IF #0 ------- 
	B constructor for : 3
		A constructor
		A constructor
		A constructor
------- END IF ------- 
	------- B destructor ------- 
		A destructor, value being deleted: 2
		A destructor, value being deleted: 1
		A destructor, value being deleted: 0


------- START IF #1 ------- 
	B constructor for : 3
		A constructor
		A constructor
		A constructor
------- END IF ------- 
	------- B destructor ------- 
		A destructor, value being deleted: 2
		A destructor, value being deleted: 1
		A destructor, value being deleted: 0


------- START IF #2 ------- 
	B constructor for : 3
		A constructor
		A constructor
		A constructor
------- END IF ------- 
	------- B destructor ------- 
		A destructor, value being deleted: 2
		A destructor, value being deleted: 1
		A destructor, value being deleted: 0
...

so you can see every second B being allocated, which in turns allocate 3 A, then when the IF is terminated, B destructor is called on b (as it was a local variable) which calls delete [] on the array of A instances and you see the 3 of them being destructed (interesting to see that it starts by the end of the array).

it's always good to be careful with dynamic memory allocation, here we don't have any check to see if the allocation has actually worked and so you might get in trouble if asking too much memory.
Also playing with dynamic allocation in the wrong way can poke holes in the heap thus over time leading to your application crashing. Here I free up memory blocks in the opposite order they were allocated so that should not poke any holes, the heap when exiting the if() statement will look like the same way it was when I entered it.

1 Like