Interface und Defaultimplementierung trennen - Merfachvererbung?

[Gast1337]

Hallo zusammen,

ich habe folgende Situation:
Ich will ein paar Interfaces festlegen, um darauf auf Objekte zuzugreifen und außerdem möchte ich Defaultimplementierungen bereitstellen (damit sind auch Datenelemente gemeint). Aber trotzdem will ich die Defaultimplementierung vom eigentlichen Interface trennen um es dennoch zu ermöglichen das Interface zu implementieren, ohne die default-Datenelemente zu erben.
Das Problem tritt auf, wenn ich von dem Interface weitere Interfaces ableite, aber trotzdem die Defaultimplementierung für ein abgeleitetes Interface nutzen will, das abgeleitete Interface aber die Schnittstelle vom Basisinterface erbt, so dass ich dann teile der Schnittstellen doppelt erbe, ich bin mir nicht sicher, ob das ein Problem ist, oder ob das egal ist.

Also folgendes dient als Beispiel:

//reines Interface
class Movable
{
public:
  Rect const getBoundRect() const = 0;
  Vector2 const getSpeed() const = 0;
  void Move(float dx, float dy) = 0;
  //....
  //keine Datenelemente!
};

Dannm eine Defaultimplementierung:

class DefaultMovable : public Movable
{
   public:
     Rect const getBoundRect() const
     {
        return myBoundRect;
     }
     //...
   protected:
     Rect myBoundRect;
     Vector2 mySpeed;
     Vector2 myAcceleration;
     //...
};

Sollte ich hier besser private nehmen und per getter/setter (die getter könnten public sein, die setter eher protected) drauf zugreifen, oder ist das mit protected so gut?
Ich bin eigentlich fürs protected, aber würde gerne eure Meinungen hören.

Jetzt gibt es ein von Movable abgeleitetes Interface:

class Enemy: public Movable
{
   public:
     //mir fällt gerade nichts besseres ein...
     void doLogics(float delta) = 0;
};

und dann will ich jetzt einen konkreten Enemy implementieren und die Standardimplkementierung für Movable nutzen:

class EvilEnemy23 : public Enemy, /*public / private / protected??*/ DefaultMovable
{
  public:
     void doLogics(float delta)
     {
        //mach irgendwas tolles (ja, ich weiß, dass sowas normalerweise besser in ne cpp datei kommt)
     }
};

So, also die Hauptfragen sind dann:
- Welche Art der Vererbung ist hier angebracht?
- Gibt es irgendwelche Probleme wegen mehrfachvererbung?
- Gibt es Alternativen zu diesem Design
- Ich möchte allgemein eure Meinungen hören bitte

[dot]

Ich denke was du suchst ist eine virtuelle Basisklasse:

//reines Interface
class Movable
{
public:
  Rect const getBoundRect() const = 0;
  Vector2 const getSpeed() const = 0;
  void Move(float dx, float dy) = 0;
  //....
  //keine Datenelemente!
};

class DefaultMovable : public virtual Movable
{
   public:
     Rect const getBoundRect() const
     {
        return myBoundRect;
     }
     //...
   protected:
     Rect myBoundRect;
     Vector2 mySpeed;
     Vector2 myAcceleration;
     //...
};

class Enemy: public virtual Movable
{
   public:
     //mir fällt gerade nichts besseres ein...
     void doLogics(float delta) = 0;
};

class EvilEnemy23 : public Enemy, public DefaultMovable
{
  public:
     void doLogics(float delta)
     {
        //mach irgendwas tolles (ja, ich weiß, dass sowas normalerweise besser in ne cpp datei kommt)
     }
};

Warum muss Enemy ein Movable sein?

[Gast1337]

Danke für die Antwort.
Zu der virtuellen Vererbung muss ich mir erstmal angucken, wie genau das funktioniert.

Zu der Frage, warum ein Enemy ein Movable sein muss:

Ich verwalte in meinem Spiel alle Objekte in vectoren mit pointern auf Basisklassen.
Die Basisklassen müssen einiges bereitstellen an Interfaces damit ich damit auch etwas machen kann, wie Kollisionserkennung (wofür ich z.B. das BoundingRect brauche).
Und Enemies neigen nunma dazu sich zu bewegen, deswegen finde ich es nicht abwegig, dass ein Enemy ein Movable sein muss.
Wenn dann könnte man jetzt argumentieren, dass getBoundingRect nicht unbedingt in Movable drin sein muss, aber dann würde ich die hierarchie halt ein wenig ändern.
Auf jeden Fall ist es wichtig, dass z.B. enemy einiges an Methoden bereitstellt, wie getBoundingRect, getSpeed und mehr.

[brotbernd]

Man kann das Problem wie gesagt mit virueller Vererbung lösen. Es ist aber ein schönes Beispiel wieso man eben möglichst wenig Implemetierung vererben sollte. Das ganze wird im Zusammenhang mit Mehrfachvererbung recht komplex und Änderungen an der Implementierung sind schwer zu überblicken (Fragile Base Class Problem).
Eine Lösung ohne Vererbung wäre hier die DefaultMovable Klasse unabhängig von der Movebale Schnittstelle zu machen und konkrete Moveable Klassen diese Implementierung über Komposition wiederverwenden zu lassen. Das ist zwar mehr Schreibarbeit, weil EvilEnemy23 jetzt die Moveable Schnittstelle implementieren muss, allerdings mit trivialem Inhalt, nämlich dem entsprechenden Aufruf an seine DefaultMovable Instanz. Triviale Schreibarbeit stellt aber den geringsten Aufwand beim Programmieren un Warten dar, darum sollte man Codewiederverwendung besser über Komposition lösen.

class Moveable
{
public:
  virtual void Move() = 0;
};

class Enemy
{
public:
 virtual void DoLogic() = 0;
};

class Mover
{
public:
  void Move();
  MoveData data;
};

class MovingEnemy : public Enemy, public Moveable
{
public:
  void Move()
  {
    mover_.Move();
  }
  void DoLogic();
private:
  Mover mover_;
};

[bmario]

Im Meyers gibt's dazu auch ein Kapitel. Da war das etwa so gelöst:

class Moveable
{
public:
  virtual void move(int km) = 0;

};

void Moveable::move(int km)
{
  // do something...
  std::cout << km << "Kilometer gefahren." << std::endl;
}

class Car : public Moveable
{

};

Car bmw;
bmw.move(24); // Fehler! Keine Implementierung angeben

class Convertible : public Moveable
{
public:
  virtual void move(int km)
  {
    Moveable::move(km); // Standardimplementierung explizit aufrufen
  }
};

Convertible slk;
slk.move(24) // nutzt Moveable::Move()

class Bmx : public Moveable
{
public:
  virtuel void move(int km)
  {
    if(km > 10) std::cout << "Das ist zu weit!" << std::endl;
    else std::cout << km << " Kilometer geradelt." << std::endl;
  }
};

Bmx b;

b.move(24); // nutzt Bmx::move()

Ich hoffe mal das stimmt so, hab den Meyers grade nicht zur Hand.

[Tachyon]

Man kann das auch wie folgt lösen:

//das interface
struct an_foobar_interface
{
    virtual void foo() = 0;
    virtual float bar(int) = 0;
    virtual ~an_interface(){}
};

//ein interface welches an_foobar_interface erweitert
struct an_special_interface : public an_foobar_interface
{
    virtual do_special() = 0;
    virtual ~an_special_interface{}
};

//standard implementierung
class foobar_std_impl
{
public:
    //non virtual!
    void foo(){...}
    float bar(int i){...}
private:
    //private data
};

//fuegt die standardimplementierung hinzu
//keine Ahnung, wie man das nennt. statisches mixin?
template<typename FooBarIF>
class add_foobar_std_implementation : public FooBarIF
{
public:
    static_assert((is_base_of<an_foobar_interface, FooBarIF>::value), "FooBarIF has to be derived from an_foobar_interface");
    virtual void foo(){ m_impl.foo(); }
    virtual float bar(int i){ return m_impl.bar(i); }
private:
    foobar_std_impl m_impl;
};

//some_implementer *ist ein* an_foobar_interface und ein an_special_interface
//und bekommt eine statndardimplementierung für an_foobar_interface
class some_implementer : public add_foobar_std_implementation<an_special_interface>
{
    //...
};

Keine Ahnung wie man das nennt. Auf jeden Fall kommt man ohne (virtuelle) Mehrfachvererbung aus.

[RHBaum]

Wichtiger Leitsatz bei Vererbung:

Vererbe nicht um wiederzuverwenden, sondern um wiederverwendet zu werden.

Sprich Vererbung um Code tippen einzusparen -> boeses Ansinnen
Vererbung um Klassen zur Laufzeit austauschen zu koennen -> gutes ansinnen.

Dein Ansinnen mit dem DefaultMove als Teil der Hirarchie ist daher designtechnisch eher ne sehr schlechte Idee.

DefaultMovable - das Interface bringt dir keine Methode und Nix, sollte damit ueberhaupt kein Interface sein.
Und ist nur in der Hirarchie um Dir Tipparbeit zu sparen.

Sauberer Ansatz.
Das DefaultMovable aus der Hirarchie raus ...

An einem Interface Methoden implementieren find ich auch unsauber.

Besser: DefaultMovable als Implementationsklasse behalten.
bei bedarf in den Implementierungen instanziieren -> Aggregation, das Mittel der Wahl fuer Codewiederverwendung !
und die Methoden alle an das Ding weiterleiten !

Ist mehr schreibarbeit, aber haelt deine Hirarchien uebersichtlicher, was imens wichtiger ist !

Ciao ...

[dot]

Ich finde das Problem hier ist dass man sich auf den Gedanken versteift dass ein Enemy ein Movable sein muss. Jetzt wo das ganze Design drumherum auf dieser Annahme aufbaut ist es natürlich sehr aufwändig das zu ändern, aber wenn du mich fragst wären Enemy und Movable erstmal zwei völlig orthogonale Konzepte. In einem Design das nicht von dieser (imo fehlgeleiteten) Annahme ausgeht würden all die hier diskutierten Probleme von vornherein nicht auftreten...

[Gast1337]

Vielen Dank für eure Vorschläge!

Ich glaub ich werde es dann so machen, dass ich die Defaultimplementierungen nicht in die Hierarchie reintue, sondern ohne Basisklasse mache und dann bei bedarf durch Aggregation benutze.
Dazu dann noch die Frage:
Sollte ich die Datenelemente der Defaultimplementierung dann public machen?
Ich will / muss evtl. für erweiterte logik darauf zugreifen.
Oder ich könnte getter/setter nehmen.

Auch das Enemy nicht von Movable erben muss habe ich mir nochmal überlegt, allerdings sehe ich da noch ein Problem:
Ich demonstriere das mal an etwas anderen Klassen:

class Collidable
{
public:
   virtual Rect const getBoundingRect() const;
   //je nach typ des anderen objekts wird das richtige collideWith verwendet
   virtual void collideWith(Enemy& e);
   virtual void collideWith(Obstacle& o);
   //mehr kollisionen nach typen
   virtual ~Collideable(){}
};

class Enemy /*public Collidable*/
{
   //irgendwelche neuen methoden, bin mir aber nicht 100% sicher, ob ich da überhaupt neue brauche
   virtual ~Enemy(){}
}

Also dann beim Enemy interface bin ich mir nichtmal sicher, ob ich da überhaupt neue Methoden brauche, aber ich brauche das Interface trotzdem damit collideWith je nach Typ das richtige tut.

Ich verwende dann mehrere typsichere container mit pointern auf Basisklassenelemente, mit Basisklassen ist hier sowas wie Enemy gemeint.
Und dann will ich das ungefähr so benutzen:

vector<Enemy*> enemies;
vector<Obstacles*> obstacles;
//...

//einfaches beispiel
for(vector<Enemy*>::iterator it = enemies.begin(), end = enemies.end(); it != end; ++it)
{
   //getBoundingRect bei enemy nicht vorhanden, also würde das nicht gehen...
   if((*it)->getBoundingRect().intersects(otherObject.getBoundingRect()))
   {
       //starte kollisionen
       (*it)->collideWith(*obstacle);
       obstacle->collideWith(*it);
   }
}

Noch so am Rande:
Ist das verwenden von iteratoren hier eigentlich besserer stil als mit ner zählvariable?
Und wie ist es von der Geschwindigkeit? (ich weiß, dass das nichts ausmacht, interessiert mich nur mal)

Ich bitte um weitere Tipps

[brotbernd]

Daten öffentlich machen nur wenn es reine Datensammlungen sind, sprich PODs. Enthält die Klasse/struct Logik, dann Daten privat und Getter/Setter nur wenn sie wirklich gebraucht werden. Die DefaultMoveable Klasse ist eine Klasse wie jede andere auch.
Ob dein Enemy ein Moveable ist musst du dir selbst beantworten. Wenn alle Enemies bewegt werden sollen und das über das Enemy Interface, dann ist Enemy ein Moveable. Das kann durchaus Sinn machen, vielleicht aber auch nicht.

Iteratoren sind ein schönes Konzept um den Zugriff auf Daten zu abstrahieren und sind daher i.A. vorzuziehen. Über Performanceunterschiede wird hier gelegentlich diskutiert. Wenn dein Compiler, wie z.B. der MSVC in der Standardeinstellung irgendeinen Schnickschnack um einen einfachen vector::iterator baut, dann ist der natürlich langsamer. Man kann im Fall des MSVC dem Compiler aber auch sagen, dass er das nicht soll (#define _SECURE_SCL 0), dann ist das gleich schnell.

[Gast1337]

Ok, ich hab jetzt mal mit der Defaultimplementierung außerhalb der Hierarchie gamacht, sieht jetzt so aus:

class GameObjectDefaultImplementer
{
public:
	GameObjectDefaultImplementer(const sf::FloatRect& boundRect, 
		const sf::Vector2f& speed, const sf::Vector2f& maxSpeed, 
		const sf::Vector2f& acceleration, XDirection::XDirection xDir,
		YDirection::YDirection yDir);
	~GameObjectDefaultImplementer(void);
	const sf::FloatRect getBoundingRect() const;
	const sf::Vector2f getSpeed() const;

	void setXDirection(XDirection::XDirection xDir);
	void setYDirection(YDirection::YDirection yDir);
	void setSpeed(const sf::Vector2f& speed);
	void setAcceleration(const sf::Vector2f acceleration);
	void setPosition(const sf::Vector2f pos);

	void update(float delta);
private:
	sf::FloatRect myBoundingRect;
	sf::Vector2f mySpeed;			//speed vorzeichenbehaftet
	sf::Vector2f myMaxSpeed;		//maxspeed absolut
	sf::Vector2f myAcceleration;	//acceleration absolut
	XDirection::XDirection myXDir;  //Richtung in die beschleunigt wird
	YDirection::YDirection myYDir;  //Richtung in die beschleunigt wird

	void move(float dx, float dy);
	static void capSpeed(float &speed, float max);
};

Ich werde das dann per aggregation benutzen und dann sehe ich ja, ob das irgendwelche probleme macht, oder die perfekte lösung ist