0

I want a derived class to be able to copy from another class if it has the same type, if not just do nothing.

Example in semi-pseudocode:

class Animal
{
public:
  virtual void CopyFrom(Animal* animal) = 0;

  // TODO: virtual destructor
};

class Dog : Animal
{
public:
  Dog() {}

  void CopyFrom(Animal* animal) override {
    if (animal is Dog) {
      likes_to_bark = ((Dog)animal).likes_to_bark;
    }
  }

private:
  bool likes_to_bark;
}

I know this can be achieved a dynamic cast, but that is considered code smell. I was looking into the visitor pattern and double dispatch, but I could not find a way to do this. Any ideas?

Moe
  • 1
  • 1
  • What should happen if `animal` is not a dog? Is every animal copyable to every other? Because that is what `CopFrom` signature claims. – Quimby Oct 25 '22 at 07:35
  • `I know this can be achieved a dynamic cast, but that is considered code smell` Why is it considered "code smell"? – The Dreams Wind Oct 25 '22 at 07:42
  • @TheDreamsWind *"Why is it considered 'code smell'"* -- because `dynamic_cast` is often abused, perhaps abused more often than used well. It's not always wrong, but if you play the odds... – JaMiT Oct 25 '22 at 07:54
  • 1
    @JaMiT I see nothing wrong with using it for testing type conformance though. – The Dreams Wind Oct 25 '22 at 09:27
  • Relevant question: [virtual assignment operator C++](https://stackoverflow.com/q/669818/580083). Note that the solutions proposed there are based on `dynamic_cast`. I don't think there is any code smell with using it here. – Daniel Langr Oct 25 '22 at 09:34
  • @TheDreamsWind You might note the "It's not always wrong" in my comment... I wasn't commenting on the value of `dynamic_cast` in this specific situation, but responding to why it is considered "code smell" as a general principle. You cannot invalidate a rule of thumb by pointing out a single case where it fails. In fact, if there was not a case where it fails, "smell" would be an inaccurately weak description. – JaMiT Oct 26 '22 at 03:27
  • @DanielLangr *"I don't think there is any code smell with using it here."* -- "code smell" is a rule of thumb, independent of how it is used. I think a more accurate way to express this would be "I think using it here is good, despite the smell." – JaMiT Oct 26 '22 at 03:29
  • @JaMiT Thanks for the explanation, I am not a native speaker. This _"I think using it here is good, despite the smell."_ sound better :) – Daniel Langr Oct 26 '22 at 06:31
  • @JaMiT there is no violation of any rule of thumb right here. `dynamic_cast` is designed for these specific cases, and using it here is consistent (is there any rule that prohibits using `dynamic_cast` at all?). "code smell" indicates a deeper problem in the design, when something is exploited for goal not exactly relevant or consistent for some specific scenario. I.e. if the class `Dog` had other derived classes which were made in a way that make `dynamic_cast` solution stop working here (and e.g. require you to use `typeid(...).name()` comparison to find exact type) then this would "smell" – The Dreams Wind Oct 26 '22 at 07:14
  • @TheDreamsWind *"'code smell' indicates a deeper problem in the design"* -- I disagree. For me, "code smell" indicates **the possibility of** a (deeper) problem in the design. Not a definite problem, but a sign that a second look is warranted. Code can smell, but still be good. – JaMiT Oct 26 '22 at 07:37

2 Answers2

1
 if (animal is Dog)

C++ has a solution for you: RTTI. Unless disabled, it allows dynamic_cast to cast a down-pointer (to parent type) to an up-pointer (to derived type) and returns a null pointer if the cast isn't possible.

if (dynamic_cast<Dog*>(animal)) {
    // this bloc is only executed if animal is indeed a Dog
}

Additionally, you could tweak and rename your function copyFrom to operator=, allowing nice user syntax:

Dog& operator=(Dog const& other) = default // or define it if it has specificities
    
Dog& operator=(Animal const& animal)
{
    Dog const* other = dynamic_cast<Dog const*>(&animal)
    if (other) {
        *this = *other;
    }
    return *this;
}

int main()
{
    Animal spider;
    Dog pluto, laika;
    Animal& animal = pluto;

    laika = spider; // does nothing
    laika = animal; // copies pluto to laika
}

I know this can be achieved a dynamic cast, but that is considered code smell.

A code-smell is what it is: a smell. I'd rather see a simple code using dynamic_cast in a proper manner that something trying to hide it uses casting under the hood in a convoluted way.
The tool is available, it can be used. Just avoid abusing it.

YSC
  • 38,212
  • 9
  • 96
  • 149
0

rtti is of course necessary but you can achieve this readily in the base:

class Animal
{
public:
  virtual void CopyFrom(Animal* animal){
     if(typeid(this) == typeid(animal){
         // *this = *animal; // A virtual assignment
         // Or better, a pure virtual (private) method defined in Animal.
         copy(animal)  
     }
  }
};

The advantage over dynamic_cast is that this behavior can be expressed in the base class.

Captain Giraffe
  • 14,407
  • 6
  • 39
  • 67