OOP Constructors as Parameters

Projects Programming
1

Hi,

I'm new here and also to C++. My background is Ruby, which simplifies everything possible. So in one hand, I had not such a hard start with basic concepts in C++. But for me not so basic concepts as pointers drive me crazy (Not so basic for me coming from a higher level language).
Reading for quite some time now in this forum and found a lot of helpful information.
Right now I'm hanging on a problem mit constructors.
I tried a lot of different approaches and none of them worked at the end. (Compiled but didn't gave me the right result).

I'm trying to code some logic for a robot. I try the OOP approach and tried to encapsulate a lot of logic in specific classes:

Body
|- Leg
   |-Joint

I combine the cpp and h files for simplicity:
Body:

#define NUM_LEGS 2

#include <Arduino.h>
#include <Adafruit_PWMServoDriver.h>
#include "Leg.h"
#include "Head.h"

class Body {
private:
    Adafruit_PWMServoDriver pwm = Adafruit_PWMServoDriver(0x40, Wire);
public:
    Leg legs[NUM_LEGS];
    Head head;

    Body();
    void begin();
    void nextStep();
};

Body::Body(){
    legs[0] = Leg(Joint(pwm, 0), Joint(pwm, 1), Joint(pwm, 2));
    legs[1] = Leg(Joint(pwm, 3), Joint(pwm, 4), Joint(pwm, 5));
}

void Body::begin(){
    pwm.begin();
    pwm.setOscillatorFrequency(27000000);
    pwm.setPWMFreq(SERVO_FREQ);
    for(int i = 0; i < NUM_LEGS; i++){
        legs[i].begin();
    }
}

void Body::nextStep(){
    for(int i = 0; i < NUM_LEGS; i++){
        legs[i].nextStep();
    }
}

#include <Arduino.h>
#include "Joint.h"

class Leg {
private:
    Joint _joint1;
    Joint _joint2;
    Joint _joint3;

public:
    Leg();
    Leg(Joint joint1, Joint joint2, Joint joint3);
    void begin();
    void nextStep();
};

Leg::Leg(){}

Leg::Leg(Joint joint1, Joint joint2, Joint joint3){
    _joint1 = joint1;
    _joint2 = joint2;
    _joint3 = joint3;
}

void Leg::begin(){
    _joint1.begin();
    _joint2.begin();
    _joint3.begin();
}

void Leg::nextStep(){
    SerialUSB.println(_joint1._number);
}

#include <Arduino.h>
#include <Adafruit_PWMServoDriver.h>
#include <Servo.h>

class Joint {
private:
    Adafruit_PWMServoDriver _pwm;

public:
    int _number;
    Joint();
    Joint(Adafruit_PWMServoDriver pwm, int number);
    void begin();
};

Joint::Joint(){}

Joint::Joint(Adafruit_PWMServoDriver pwm, int number){
    _pwm = pwm;
    _number = number;
}

void Joint::begin(){
}

This is the basic classes I use. I Initialize a Body in global scope, running body.begin() in setup and body.nextStep() in loop.
My console starts to output a mostly really big random number. I was expecting to see 0 and 3 in loop.

My intuition tells me, there is a problem in the construction or the "Reference". It's pretty new to me not seeing a exception and receive random numbers and still "running" programs.

So how do you construct such a example in C++ with working references?

I think it would be too long, when I post all the code I tried yet, so I try to explain them:

  1. Changed Leg-Constructor: Leg::Leg(Joint* joint1, ...); Joint* _joint1;
    In Body: Joint *j = new Joint(pwm, 0); legs[0] = Leg(j, ....);
  2. Changed assignment: Leg::Leg(Joint joint1, ...) : _joint1(joint1), ... {
  3. Changed Leg-Constructor: Leg::Leg(Joint &joint1, ...){

Nothing keeps to work. There are always random numbers in the output.
So how to create an object and assign it to the constructor of another object?

Thanks in advance.

2

Try to run this (typed here, totally untested) with the serial monitor opened at 115200 bauds.

const byte NUM_LEGS = 2;

class Joint {
  public:
    int _number;
    Joint(int number) : _number(number) {
      Serial.print("\t\t\tcreating Joint #"); Serial.println(_number);
    }
    ~Joint() {
      Serial.print("\t\t\tdeleting Joint #"); Serial.println(_number);
    }
    void begin() {}
};

class Leg {
  private:
    Joint* _joint1;
    Joint* _joint2;
    Joint* _joint3;

  public:
    Leg(int j1, int j2, int j3) {
      Serial.println("\t\tcreating Joints");
      _joint1 = new Joint(j1);
      _joint2 = new Joint(j2);
      _joint3 = new Joint(j3);
      Serial.println();
    }

    ~Leg() {
      Serial.println("\t\tdelete Joints");
      delete _joint1;
      delete _joint2;
      delete _joint3;
      Serial.println();
    }

    void begin() {
      _joint1->begin();
      _joint2->begin();
      _joint3->begin();
    }

    void nextStep() {
      Serial.println("nextStep()");
      Serial.println(_joint1->_number);
      Serial.println(_joint2->_number);
      Serial.println(_joint3->_number);
      Serial.println();
    }
};

class Body {
  public:
    Leg* legs[NUM_LEGS];

    Body() {
      Serial.println("\tcreating Legs");
      legs[0] = new Leg(0, 1, 2);
      legs[1] = new Leg(3, 4, 5);
    }


    ~Body() {
      Serial.println("\tdelete Legs");
      for (auto && l : legs) delete l;
    }

    void begin() {
      for (auto && l : legs) l->begin();
    }

    void nextStep() {
      for (auto && l : legs) l->nextStep();
    }
};


void setup() {
  Serial.begin(115200);
  Serial.println("creating Body");
  Body body;
  body.begin();
  body.nextStep();
  // here Body gets deleted as it's local to the function so you'll see messages from the destructors
  Serial.println("deleting Body");
}

void loop() {}

if all goes well you should see messages about creating the Body instance which in turn will instantiate the Legs and the Joints

then nextStep() will be called on the body which will cascade to the joints

then the setup() exits and thus the body instance gets deleted which will trigger all the destructors for your embedded instances

give it a try!

3

There is no need to use new here. Usage of new should be limited to low-level code only.
C++ Core Guidelines R.11: Avoid calling new and delete explicitly
The Leg and Body classes also violate the rule of 3/5/0 and will corrupt memory:
C++ Core Guidelines C.21: If you define or =delete any copy, move, or destructor function, define or =delete them all

If you ever (explicitly or implicitly) make a copy of a Leg, for example, you're in for a bad time:

    Leg l1(0, 1, 2);
    Leg l2 = l1;

==1==ERROR: AddressSanitizer: attempting double-free on 0x6020000000d0 in thread T0:
    #0 0x7f83f4d72dd7 in operator delete(void*, unsigned long) (/opt/compiler-explorer/gcc-11.2.0/lib64/libasan.so.6+0xb3dd7)
    #1 0x401b05 in Leg::~Leg() /app/example.cpp:48
    #2 0x4015c4 in main /app/example.cpp:111
    #3 0x7f83f47760b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #4 0x4011fd in _start (/app/output.s+0x4011fd)

Avoid all these issues by just using “the rule of zero” (i.e. not doing any resource management in custom destructors at all):

const uint8_t NUM_LEGS = 2;

class Joint {
  public:
    int number;
    Joint(int number) : number(number) {
      Serial.print("Joint::Joint("); Serial.print(number); Serial.println(")");
    }
    ~Joint() {
      Serial.print("Joint::~Joint("); Serial.print(number); Serial.println(")");
    }
    void begin() {
        Serial.print("Joint::begin("); Serial.print(number); Serial.println(")");
    }
};

class Leg {
  private:
    Joint joint1;
    Joint joint2;
    Joint joint3;

  public:
    Leg(Joint joint1, Joint joint2, Joint joint3)
        : joint1(joint1), joint2(joint2), joint3(joint3) {
      Serial.print("Leg::Leg(");
      Serial.print(joint1.number); Serial.print(", ");
      Serial.print(joint2.number); Serial.print(", ");
      Serial.print(joint3.number); Serial.println(")");
    }

    void begin() {
      joint1.begin();
      joint2.begin();
      joint3.begin();
    }

    void nextStep() {
      Serial.print("Leg::nextStep(");
      Serial.print(joint1.number); Serial.print(", ");
      Serial.print(joint2.number); Serial.print(", ");
      Serial.print(joint3.number); Serial.println(")");
    }
};

class Body {
  public:
    Leg legs[NUM_LEGS] {
        {0, 1, 2},
        {3, 4, 5},
    };

    void begin() {
      for (Leg &l : legs)
        l.begin();
    }

    void nextStep() {
      for (Leg &l : legs)
        l.nextStep();
    }
};


void setup() {
  Serial.begin(115200);
  Serial.println("Creating Body");
  Body body;
  body.begin();
  body.nextStep();
  // here Body gets deleted as it's local to the function so you'll see messages from the destructors
  Serial.println("Deleting Body");
}

void loop() {}
4

yes this is true. Lame excuse is that It was beyond the scope of this simple example typed here (ie I've been lazy as typing in the browser).

What would be the suggestion if the number of legs or joints was dynamic ?

Rule R11 can be discussed as the instance does belong to a resource that can call delete.

side not regarding your code:

Isn't that going to invoke a copy & delete that is not necessary

which should be visible if you add an explicit copy constructor to the Joint class

    Joint(const Joint &j) {Serial.print("Duplicating Joint "); Serial.println(number = j.number);}

I would suggest to do

Leg(int j1, int j2, int j3) 
  : joint1(j1), joint2(j2), joint3(j3) {

instead

5

A std::vector member variable (or alternative if you're on AVR), which allows following the rule of zero. It's good practice to keep memory management and usage separate: i.e. a Body class would be concerned with controlling its legs, not with managing their memory.

True, but I think the part about "a naked delete (that is a delete in application code, rather than part of code devoted to resource management) is a likely bug" does apply here.

Yes, but if it's just an integer (or any plain struct) it doesn't matter.

Here you're copying three integers, in my example I'm copying three integers wrapped in a struct, which is the same. If you add non-trivial copy constructors or destructors, the compiler has to call these.

If Joint is a more complex type, e.g. containing a String, you would probably pass it by const ref:

Leg(const Joint &joint1, const Joint &joint2, const Joint &joint3)
        : joint1(joint1), joint2(joint2), joint3(joint3) {

Or if you want to allow stealing resources of temporaries, use rvalue references:

Leg(Joint &&joint1, Joint &&joint2, Joint &&joint3)
        : joint1(std::move(joint1)), joint2(std::move(joint2)), joint3(std::move(joint3)) {

If you want to allow combinations, it gets messy ...

I left out the references for simplicity and because it doesn't matter in this case. However, using an int argument for each joint doesn't generalize if a joint has multiple fields instead of one, so I used Joint by value instead of ìnt.

6

fair - not always the most effective on small AVR.

indeed - I was more thinking in terms of OP's needs where Servos might get allocated etc

7

It depends, in the majority of cases you don't really lose anything by wrapping your memory management in a vector-like container.

It looks like the Adafruit_PWMServoDriver constructor doesn't do anything beyond storing the arguments to member variables, so in this case it doesn't really matter. If you're working with poorly designed libraries that try to access hardware in constructors, you're of course right, and it might be better to construct those objects in-place rather than passing them to the constructor as an argument and using the copy-constructor.

8

I would build things from the bottom up:

#include "Joint.h"

Joint LeftHip(0);
Joint LeftKnee(1);
Joint LeftAnkle(2);
Joint RightHip(3);
Joint RightKnee(4);
Joint RightAnkle(5);

#inlcude "Leg.h"

Leg LeftLeg(LeftHip, LeftKnee, LeftAnkle);
Leg RightLeg(RightHip, RightKnee, RightAnkle);

#include "Body.h"

Body body(LeftLeg, RightLeg);
9

Yes could do but that exposes all the details OP might have wanted to hide (ie just instantiate a body and you are done, no need to know what’s underneith)

10

Ok i had not looked - the class name sounded hardware related

11

Hi together. Thank you for all the replies. Your answers look really complicated to me. It looks like they are dive deep into the interna of the language. Still I learned something and it helped me to understand the lifecycle of a object a bit more.
A question for my understanding: Is it necessary to clean/delete the objects when they are created at the initialization phase of the project and afterwards only used? I mean as soon as I gonna shut down the arduino, the program stops anyway and RAM is empty again.

At the end I was able to solve my problems by changing:

Joint(...){
  _number = number
}

to

Joint(...) : _number(number){}

I didn't know this is different. I thought it is just another way to write it.

So it seems to work by passing the objects correctly as I was hoping.

J-M-L
Yes could do but that exposes all the details OP might have wanted to hide (ie just instantiate a body and you are done, no need to know what’s underneith)

Exactly this is what I try to achieve by OOP. It's just not that simple with some hardware-components.
I also tried to give a reference of Body to the Joints. This way the joints could grab the pwm for themself. But I think this would end up in a bigger mess. Plus it would increase coupling even more.

12

If you use new then you need to delete otherwise You’ll leak memory possibly

variables objects will be discarded when they get out of scope (global variables don’t get out of scope) and when you reboot you start from a clean slate

13

This topic was automatically closed 120 days after the last reply. New replies are no longer allowed.