0

I have a tool that's been subclassed. A bug has been reported where the subclass has taken a value from the superclass it shouldn't have taken: mySuperclass.SetMyVar() is being called for a subclass object.

If I can't make SetMyVar private – because it is being called in initialisation that happens for both classes – can I make the behaviour dependent on whether this is a superclass item or subclassed item?

The fix suggested – to supply SetMyVar() for the subclass – presents maintenance issues should the superclass be derived from again.

[Still, if I used b.SetMyVar() and it didn't do it then I might have another bug to contend with... maybe the subclass implementation is right.]

Something like this:

class A
{
public:
    A() {myVar = 20; }

    void SetMyVar(int val)
    {
        //eg (typeid (this).name() == "class A*") // doesn't work
        //eg (dynamic_cast <A*>(this)) // doesn't work

[something here to make it happen for only objects of type superclass]
        {
            myVar = val;
        }
    }

    int myVar;
};


class B : public A
{
public:
    B() { myVar = 10; }
};

int main(...)
{
    A a;
    B b;

    a.SetMyVar(2);
    b.SetMyVar(3); < -- this should not set myVar 

    std::cout << a.myVar << " : " << b.myVar;

    return 0;
}
Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
craker
  • 67
  • 9
  • 2
    What do you mean by "< -- this should not set myVar"? If it wouldn't, `B` would break Liskov's substitution principle and that basically means `B` shouldn't inherit from `A` at all, because you can't say `B` is-a `A`. – Yksisarvinen Apr 18 '23 at 12:31
  • 1
    there is no `DoStuff` method in the code – 463035818_is_not_an_ai Apr 18 '23 at 12:34
  • Do you want private inheritence, `class B : private A`? This would make public members of `A` private in `B`. – joergbrech Apr 18 '23 at 12:34
  • 1
    What does "the subclass has taken a value from the superclass it shouldn't have" mean? – molbdnilo Apr 18 '23 at 12:34
  • 3
    It would make it easier to understand if the code you show (including the naming) matches the description. (e.g. there is no `DoStuff ` in the code and nothing is shown where it is used during initintialization). Also `where the subclass has taken a value from the superclass it shouldn't have` is not reflected in your example code. – t.niese Apr 18 '23 at 12:35
  • For now, your 2 classes are not dynamically distinguishable. Add a virtual destructor in `A`, your tests will give other results. – dalfaB Apr 18 '23 at 12:45
  • `this should not set myVar` What should it set then? `I can't make DoStuff private - because it` protected? – KamilCuk Apr 18 '23 at 13:01
  • OK Sorry for the poorly formatted question / example – craker Apr 18 '23 at 13:02
  • Perhaps @Yksisarvinen is correct, B is an A so the compiler has no way of deducing that it should behave differently – craker Apr 18 '23 at 13:15
  • Programmers that are users of your class are not your enemies. Give them instructions how to use the class, and give them warnings how not to use the class. There's not much else you can or should do. – Dialecticus Apr 18 '23 at 13:20
  • Would making the method virtual in A and overriding in B help? – Faxter Apr 18 '23 at 13:27
  • @Dialecticus no, you **should not** expose a API `SetMyVar` that doesn't `set myVar`. and that's part of reason to have private member, you don't just say "don't touch this" in the doc. – apple apple Apr 18 '23 at 13:30
  • @Dialectus - agreed. Not a great use case on show here, apple apple Yes the approach is incorrect. As a coding problem, is it even possible? – craker Apr 18 '23 at 13:32
  • Does this answer your question? [When should I use C++ private inheritance?](https://stackoverflow.com/questions/656224/when-should-i-use-c-private-inheritance) – apple apple Apr 18 '23 at 13:35

1 Answers1

0

As mentioned in the comments, if you want to distinguish between a base class and a derived class dynamically (such as by using typeid), then your base class must have a virtual function.

The simplest one to add is a (default) virtual destructor; a typeid() comparison can then be used to spot a call to the base class function from a derived class object:

#include <iostream>
#include <typeinfo>

class A {
public:
    A() { myVar = 20; }
    virtual ~A() = default; // This makes the class polymorphic
    void SetMyVar(int val)
    {
    //  if (typeid(this) == typeid(A*)) {// Wrong: "this" is always of type A* here!
        if (typeid(*this) == typeid(A)) {
            myVar = val;
        }
    }
    int myVar;
};

// Remaining code as you have it ...

Note, also, that my code does the typeid check on the dereferenced this pointer (i.e. on the actual object from which the function is being called). A typeid check on the this pointer itself will always yield the type of a pointer to the class in which the code is defined.

Adrian Mole
  • 49,934
  • 160
  • 51
  • 83
  • I didn't need to make a virtual destructor for this example to work. I wouldn't have guessed to dererefence (this) to make the typeid call work - this works as requested thank you – craker Apr 21 '23 at 10:25
  • @craker Then, if your code works without the virtual destructor, I presume that your classes already have other virtual members. That'll do the same. – Adrian Mole Apr 21 '23 at 11:35