1

I have some different objects all of type Zoo, which have a member variable Collection = vector<Animal>. I have one instance of zoo where all the elements are Animals, one where all the elements are Birds, and one where all the elements are Bats. Both Birds and Bats derive from Animals.

I want there to be a method fly() that I can call on all the birds and bats, but I'm not sure the best way of doing this. Should I just cast when looping through my bird/bat zoo? Like this:

Bird thisBird = static_cast<Bird>(Collection[i]);
thisBird.fly();

... or could I somehow have a virtual function on Animal, that is only implemented on the classes that derive from it?

Ideas, and justifications for why they're 'good code', welcome!

tiswas
  • 2,041
  • 7
  • 31
  • 43
  • 2
    You are putting objects that are probably meant to be used polymorphically *by value* inside a container. That [won't end well](http://stackoverflow.com/questions/274626/what-is-the-slicing-problem-in-c). – Jon Apr 12 '13 at 11:20
  • Each zoo's `Collection` vector just has the same kind of animal in it (either generic, bird, or bat), though.. Do you have any suggestions on how I should do it if not by casting? – tiswas Apr 12 '13 at 11:23
  • ... ah - should have been Collection. Have edited to correct this – tiswas Apr 12 '13 at 11:25
  • @tiswas: That's not the point. If you have a `vector` and you push a `Bird` inside, that bird will get sliced and your vector will just hold an `Animal` that can no longer be cast back to a `Bird`. Read up on the slicing problem; the solution is to keep (smart?) pointers inside the vector. – Jon Apr 12 '13 at 11:25
  • I see.. so back to my question - what should I do instead? – tiswas Apr 12 '13 at 11:27
  • @tiswas: It depends, you have to give a high-level overview of what you are trying to achieve with enough detail. For example, do all zoos have to be of the same runtime type? Why/why not? – Jon Apr 12 '13 at 11:29
  • I guess they don't, but I thought that the most concise way of writing would be to have the as the same runtime type, rather than have different classes like `BirdZoo` and `BatZoo` which are exactly the same as `Zoo` except they have a `Vector` instead of Vector – tiswas Apr 12 '13 at 11:33

3 Answers3

1

Not all animals can fly so no point of trying to execute a specific behavior (i.e. uncommon between all animals) for a generic purpose. If you have a collection of Animal classes (or their derivatives) its for the purpose of allowing them to do a common behavior (implemented differently by each derivative class).

You can implement a FlyAble interface (i.e. abstract class) and extend it only by animals who can really fly. Than just save them in another collection designated for these type of animals (i.e. <FlyAble>).

giorashc
  • 13,691
  • 3
  • 35
  • 71
  • wouldn't that require each zoo to be of a different type, as their member variable Collection is of different types (Vector as opposed to Vector)? – tiswas Apr 12 '13 at 11:35
  • This is highly depends of what the role of your zoo in your program. And I believe if you design it correctly you will have only one zoo type in this flying context. – giorashc Apr 12 '13 at 11:49
  • Will the downvoter come out of the shadows and explain please ? – giorashc Apr 12 '13 at 14:48
1

You probably don't want a hierarchy like Bird and Bat arises from Animal without fly() method declared in lower class which are you delivering from. That way you just loose the point of unified hierarchy.

You can either do:

class Animal {
    virtual bool canFly() { return false; }
    virtual void fly() {throw Exception( "Sorry babe, I don't fly"); }
}

And override fly in Bird and Bat.

This would cause you to have method fly() implemented for dogs, cats which you probably don't want to. So maybe you could create new class Flyer which would declare this method:

class Flyer : public Animal {
    virtual void fly() = 0;
}

class Bat : public Flyer {}
class Bird : public Flyer {}

Which would be inconsistent with more detailed biological classification like Reptile, Mammal.

Another trick may be to propose method like move() and dog would implement it as run() and bird as fly(), all with unified interface.

Another thing is, that I believe that it's valid question to ask whether dog can fly, so I think method like Dog.canFly() should be implemented in your code, as a part of animal.

All this taken into account, I'd go with this:

// Your base animal
class Animal {
    virtual bool canFly() {return false;}
};

// Any animal that could fly, just abstract class
class Flyer {
    virtual void fly() = 0;
}

// Ants, flies etc.; real biological hierarchy
class Insect : public Animal {}
class Mammals : public Animals {}
class Birds : public Animals {}

// And now flying bird :
class Bird : public Birds, public Flyer {
    virtual bool canFly() {return true; }
    virtual void fly() {...}
}

// And flying insect
class Fly : public Insect, public Flyer {
    virtual bool canFly() {return true; }
    virtual void fly() {...}
}

And you just can do then (based on this answer I'm afraid that you'll have to use pointers for this):

if( Collection[i]->canFly()){
    Flyer *thisBird = static_cast<Flyer*>(Collection[i]);
}

You could also derive Flyer from Animal and use virtual inheritance, but this is called "dreaded diamond" and it's not considered good practice, but worth reading.


As pointed out by Ricibob in comment, Flyer should specify canFly(), which will in simple example:

class Flyer {
public:
    virtual bool canFly() const {return true;}
    virtual void fly() {cout << "Flying" << endl; }
};

Bird b;
cout << "Can fly: " << b.canFly() << endl;
// Error    1   error C2385: ambiguous access of 'canFly'

But if you make Flyer delivered from Animal and Bird part of classes Birds and Flyer:

class Animal {
public:
    virtual bool canFly() const {return false;}
};

class Flyer : virtual Animal {
public:
    virtual bool canFly() const {return true;}
    virtual void fly() {cout << "Flying" << endl; }
};

class Bird : virtual public Birds, virtual public Flyer {

}; 

// Warning  1   warning C4250: 'Bird' : inherits 'Flyer::Flyer::canFly' via dominance

Now canFly() returns 1, but code seems wrong to me.

At this point... You may either specify canFly() manually for each subclass (or large groups), or deliver Birds from Flyers (which isn't right for example for Chicken), or deliver new subclass:

class FlyingBrids : public Birds, public Flyer /* Flyer not delivered from Animal */ {
    virtual bool canFly() const {return true;}
};

Note that Flyer is still important because of Fly delivered from Insect.

Community
  • 1
  • 1
Vyktor
  • 20,559
  • 6
  • 64
  • 96
  • hmm I like the sound of this, but @Jon mentioned: 'If you have a vector and you push a Bird inside, that bird will get sliced and your vector will just hold an Animal that can no longer be cast back to a Bird' – tiswas Apr 12 '13 at 11:45
  • @tiswas to be honest I've always used pointers for this and those were not problematic. Just for fun, implement copy constructor and see how many times is it called. :) – Vyktor Apr 12 '13 at 11:50
  • @Vyktor Seems to make sense to move canFly default implimentation returning true into Flyer (Flyers can fly by default!) – Ricibob Apr 12 '13 at 11:57
  • @Ricibob tried to added it to answer, but it's just getting more and more complicated without one good answer. – Vyktor Apr 12 '13 at 12:19
  • That is my point "more and more complicated without.. ". Why not just add virtual void fly(){} to the base class? Why could be better add canFly()? – qPCR4vir Apr 12 '13 at 12:36
  • @qPCR4vir that's like asking why `curl` doesn't implement `download youtube video and convert it to .mkv` :) I'd go with `move()` if I could in this situation, or guessed the right "moment" when to build `Flyer` as a part of parent class, but in this example, to 1) keep biological hierarchy working; 2) prevent useless and very specific method declarations (like `fly` spreading trough 2000 kinds of viruses and bacteria) and 3) keep utilized interface of `Flyer`... it just has to be this complicated (it's just general example and you don't know how complex it will be in the end). – Vyktor Apr 12 '13 at 12:42
  • Yes, I agree with you: making a general solution is very difficult. We need some limit. The decicion is on @tiswas. For example, I dont think he want viruses and bacteria in the zoos colections ;-P – qPCR4vir Apr 12 '13 at 12:56
1

Oh, yes, Animal could have a virtual function:

virtual void move(){}
virtual void fly(){}

When you order a dog to fly it do nothing. When you order a bird to move it could call fly() too.

Define fly() correctly for the animal who fly, and just Colection[i]->fly() will do the correct thing. This way your code will be simple.

But to do it your collection have to collect pointer to the animals, not just Animal objects. For example:

#include <vector>
#include <memory>
#include <iostream>  
using namespace std;
struct Animal{virtual void fly(){}};
struct Bird:public Animal{void fly(){cout<<"fly\n";}};
int main()
{
  vector<unique_ptr<Animal>> Colection(1);
  Colection[0].reset( new Bird ); 
  Colection.push_back(unique_ptr<Animal>(new Bird) );
  Colection.push_back(unique_ptr<Animal>(new Animal) );
  Colection[0]->fly(); 
  Colection[1]->fly(); 
  Colection[2]->fly();   
}

Reading other answer I can recommend you to implement something like

struct Animal    {virtual bool fly(){return false;     }};
struct Bird:public Animal{bool fly(){cout<<"fly\n";return true;}};

And you dont need a separate canFly.

qPCR4vir
  • 3,521
  • 1
  • 22
  • 32
  • this seems like the simplest bet to me, although I'm not using C++11, so I guess I just have to have `vector Collection;` – tiswas Apr 12 '13 at 14:09