1

I have the following classes:

class ServoPart {
protected:
    virtual void doJob(byte* job) = 0;

private:
    bool moving;
    Servo servo;
};

// the following classes only have a constructor so I can use two ServoParts to inherit from
class Major: public ServoPart {};
class Minor: public ServoPart {};

class Arm: public Major, public Minor {
private:
    void move() {
        // do stuff
        if (target == current) {
            moving = false;
        }
    };

public:
    void doJob(byte* job) {/* do stuff */};
};

I can't use virtual inheritance (I think) because the Major and Minor need to control one servo each which can't be the same. However, when the Arm is done moving, it should set the moving member to false. Intellisense shows ServoPart::moving, when im typing moving.

Would this access of moving be ambiguous? If yes, how can I fix this? Is my assumption about virtual inheritance, that I can't use it because I have two different servos, correct?

7
  • 11
    Looks like a great time to consider composition over inheritance. Commented May 18, 2021 at 15:52
  • 3
    Inheritance doesn't feel like the right approach here, I'd consider composition Commented May 18, 2021 at 15:56
  • @user4581301 I used the "is a" vs "has a" method: The arm is a servo part, containing of two servos. I think for now I'll store the moving state for both, Major and Minor, and combine them when checking the status. I'll reconsider if I keep getting problems like this though Commented May 18, 2021 at 15:57
  • 1
    re doJob I updated my answer again. Having a virtual function is important information that changes the nature. Be careful not to over-simplify the sample code! Commented May 18, 2021 at 16:09
  • 1
    @JDługosz yes I can see that now. I will keep in mind for future questions. Thanks! Commented May 18, 2021 at 16:21

1 Answer 1

6

Yes, it is ambiguous and the compiler will complain. You can write Major::moving or Minor::moving within the code of Arm's member functions to specify which you want.

I question whether you have a proper "isa" relationship here. An arm is not a motor. An arm has two motors. You should be using composition here instead of inheritance. Note that Major and Minor don't have any virtual functions so there is no reason to prefer inheritance.

Try:

class Arm {
   Major major;
   Minor minor;
   void move() {
     ...
     major.moving= false;
     minor.moving= false;
     ...

It is more obvious now how to refer to the components. But with (multiple) inheritance it is the same idea. I think also you are introducing the names Major and Minor just to get around the limit of inheriting more than one copy. If you used composition, you can avoid that:

class Arm {
   ServoPart major;
   ServoPart minor;
...

Or, since they are the same type now, perhaps make an array instead. This makes it easy to "broadcast" the same operation to all of them:

class Arm {
    ServoPart joints[2];
    void move() {
        ...
        for (auto j : joints)  j.moving= false;

update

Your comment indicates ServoPart has a virtual doJob function. You should separate that into a pure interface that has no data members, and derive your single-motor class from that. Your composited class can also inherit from that interface.

struct Servo_Interface {
    virtual void doJob() =0;
};

class Joint : Servo_Interface {
    bool moving;
    Servo servo;
public:
    void doJob()  override;
};

class Arm : Servo_Interface {
    Joint joints[2];
public:
    void doJob()  override;
};

Note that the implementation of doJob in Arm can call doJob for each of the components it contains, but it is a separate function that does not have any automatic reliance on them. Both single Joint and Arm instances can be treated polymorphically since they have a common interface.

Sign up to request clarification or add additional context in comments.

3 Comments

I think I'll separately store each servos moving status then and combine both when the status needs to be checked. Thanks!
I updated the post to suggest using composition instead of inheritence.
Something I left out of the post which I should've probably added, is that the ServoPart has an abstract function doJob which takes jobs. I guess I could implement that function in Major and Minor and let it do nothing, but that wouldn't feel clean. Basically, the Arm takes a job and sets each servos position. The Major/Minor classes can't do that themselves because the position depends on the position of the other part.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.