Referenz und Wiederverwendbarkeit in Schleifen

[Reth]

Danke für Deine Antwort!

brotbernd schrieb:

Wieso nicht? Ist in diesem Fall halt notwendig. Dir muss halt klar sein, dass dein animantions Zeiger ohne default Block durchaus null sein kann.
Was mir überhaupt nicht gefällt, ist dass dort scheinbar immer der gleiche Aufruf steht, bis auf diesen GFX_... Wert. Wieso betsmmtst du nicht erst anhand field den GFX_Wert und rufst dann an einer Stelle die Create Funktionen auf. Dann hat sich auch dein Zeigerproblem gelöst.

Das mit den mehrfachen, fast identischen Aufrufen gefällt mir auch nicht und über die Lösung aus Deinem Vorschlag hab ich auch schon nachgedacht. Allerdings muss ich dann noch ein paar Ausnahmen behandeln. Hier mal der ganze Block:

// zufälliges Feld ermitteln (zw. 0 und 5)
    int field = rand()%5;
    AnimObjectC *animObject = NULL;    // Zeiger für äußeres Objekt
    AnimationC *animation = NULL;    // Zeiger für Memberobjekte

    switch (field)
    {
        case LAKE:
            animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, LAKE, FALSE, ...);
            animation = &(animObject->createAnimation(FALSE, 0, FALSE, GFX_LAKEFIELD, FALSE));
            ...
            break;

        case GRASS:
            animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, GRASS, FALSE, ...);
            animation = &(animObject->createAnimation(FALSE, 0, FALSE, GFX_GRASSFIELD, FALSE));
            ...
            break;

        case WOOD:
            animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, WOOD, FALSE, ...);
            animation = &(animObject->createAnimation(FALSE, 0, FALSE, GFX_WOODFIELD, FALSE));
            ...
            break;

        case SWAMP:
            animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, SWAMP, FALSE, ...);
            animation = &(animObject->createAnimation(FALSE, 0, FALSE, GFX_SWAMPFIELD, FALSE));
            ...
            break;

        case STONE:
            animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, STONE, FALSE, ...);
            animation = &(animObject->createAnimation(FALSE, 0, FALSE, GFX_STONEFIELD, FALSE));
            ...
            break;

        case DESERT:
// not implemented yet
	   break;
    }
    ...
    if (animObject != NULL)
    {
         ...

Es gibt also keinen Default-Block, aber eine nachgelagerte Prüfung.
Das mit dem Vorschlag im Switch/case nur den GFX-Wert zu bestimmen macht den Code deutlich schlanker, aber man sieht nur noch indirekt, dass auch die Symbols LAKE, SWAMP usw. in den Objekten zum Einsatz kommen (sind rein numerische Werte, positive ganze Zahlen und 0). Es wird nur noch der Wert von field direkt verwendet.

brotbernd schrieb:

Eigentlich würde ich sogar eher erwarten, dass die jeweilge konkrete AnimObjectC Klasse die Animation erzeugt ohne noch den Typ vorgegeben zu bekommen. AnimObjectC::createAnimation ist eine Fabrikmethode(http://en.wikipedia.org/wiki/Factory_method_pattern). LakeAnimObject erzeugt eine LakeAnimation und GrassAnimObject erzeugt eine GrassAnimation.
Da stecken jetzt aber ziemlich viele Annahmen über dein Programm drin.

FactoryPattern kenne ich z.T. schon. Du meinst hier lauter spezielle Fabriken, für jede Art von AnimObject eine? Dagegen hatte ich mich entschieden, da sich die einzelnen Objekte nur über den Typ (initialisiert über den GFX...-Wert) unterscheiden und in sonst gar nichts. Daher fand ich, dass eine Spezialisierung nichts bringt.
Eine Alternative wäre pro AnimObject ne eigene create-Methode. Finde ich aber auch unschön, da man so für neue Objekt-Typen neue Methoden einführen muss, statt nur eines neuen Symbols!

Ciao

[brotbernd]

Reth schrieb:

Danke für Deine Antwort!
Hier mal der ganze Block:

// ...

Irgendwie seh ich gar keinen Unterschied zum ersten Code. Falls es doch einen gibt: Du siehst schon, ist irgendwie unübersichtlich.

Das mit dem Vorschlag im Switch/case nur den GFX-Wert zu bestimmen macht den Code deutlich schlanker, aber man sieht nur noch indirekt, dass auch die Symbols LAKE, SWAMP usw. in den Objekten zum Einsatz kommen (sind rein numerische Werte, positive ganze Zahlen und 0). Es wird nur noch der Wert von field direkt verwendet.

Es verhindert nicht nur die Wiederholung von identischem Code, ich finde es auch noch übersichtlicher. Ich seh keinen Nutzen darin im Funktionsaufruf nochmal die Symbole LAKE, SWAMP... zu sehen. Vielelicht hilft es die Variable field besser zu benennen?

FactoryPattern kenne ich z.T. schon. Du meinst hier lauter spezielle Fabriken, für jede Art von AnimObject eine? Dagegen hatte ich mich entschieden, da sich die einzelnen Objekte nur über den Typ (initialisiert über den GFX...-Wert) unterscheiden und in sonst gar nichts. Daher fand ich, dass eine Spezialisierung nichts bringt.
Eine Alternative wäre pro AnimObject ne eigene create-Methode. Finde ich aber auch unschön, da man so für neue Objekt-Typen neue Methoden einführen muss, statt nur eines neuen Symbols!

ich meinte Letzteres: Eine Fabrikmethode.

class Animation
{
};

class AnimObject
{
public:
    virtual Animation* CreateAnimation() = 0;  // fabrik methode
};

class Lake : public AnimObject
{
public:
    class LakeAnimation : public Animation{};
    Animation* CreateAnimation()
    {
        return new LakeAnimation();
    }
};

class Grass : public AnimObject
{
public:
    class GrassAnimation : public Animation{};
    Animation* CreateAnimation()
    {
        return new GrassAnimation();
    }
};

Eine Fabrik wäre dann dein AnimObjectManager, mit dem sich AnimObject Instanzen erzeugen lassen ohne die konkreten Typen zu kennen.

class AnimObjectManager/*Factory */
{
public:
    AnimObject* CreateAnimObject(int field);
};

Finde ich aber auch unschön, da man so für neue Objekt-Typen neue Methoden einführen muss, statt nur eines neuen Symbols!

Überlege dir, welche Änderungen eine Einführung neuer Typen mit sich bringt. Eine neue Klasse hinzuzufügen ist eine Erweiterung, keine Änderung. Du musst bestehenden Code nicht anfassen. Führst du hingegen einen neuen Typen ein indem du ein neues Symbol einführst, musst du alle deine switch/case und if Abfragen um dieses Symbol erweitern. Du musst bestehenden Code ändern. Programmiere offen für Erweiterungen und geschlossen für Änderungen:
http://en.wikipedia.org/wiki/Open/closed_principle

[Reth]

brotbernd schrieb:

Irgendwie seh ich gar keinen Unterschied zum ersten Code. Falls es doch einen gibt: Du siehst schon, ist irgendwie unübersichtlich.

Die Unterschiede sind der letzte leere Fall (Desert), das fehlende default: und die Abfrage nach switch/case.

brotbernd schrieb:

ich meinte Letzteres: Eine Fabrikmethode.

class Animation
{
};

class AnimObject
{
public:
    virtual Animation* CreateAnimation() = 0;  // fabrik methode
};

class Lake : public AnimObject
{
public:
    class LakeAnimation : public Animation{};
    Animation* CreateAnimation()
    {
        return new LakeAnimation();
    }
};

class Grass : public AnimObject
{
public:
    class GrassAnimation : public Animation{};
    Animation* CreateAnimation()
    {
        return new GrassAnimation();
    }
};

Da sind die speziellen Klassen aber auch nur "Marker", da sie sich alle in ihrem Verhalten nicht unterscheiden. Im ganzen restlichen Programm werden auch alle Animationen und AnimationsObjecte gleich behandelt. Die Unterscheidung, ob SWAMP, WOOD, usw. wird nur benötigt, wenn die Ressourcen ermittelt werden. Das stell ich mir mit den speziellen Klassen wiederum schwieriger vor! Hier ist doch die Unterscheidung nach Typ zwar unschöner (bzgl. Konzepten wie OO usw.), aber pragmatische, oder?

brotbernd schrieb:

Überlege dir, welche Änderungen eine Einführung neuer Typen mit sich bringt. Eine neue Klasse hinzuzufügen ist eine Erweiterung, keine Änderung. Du musst bestehenden Code nicht anfassen. Führst du hingegen einen neuen Typen ein indem du ein neues Symbol einführst, musst du alle deine switch/case und if Abfragen um dieses Symbol erweitern. Du musst bestehenden Code ändern. Programmiere offen für Erweiterungen und geschlossen für Änderungen:
http://en.wikipedia.org/wiki/Open/closed_principle

Da geb ich Dir recht! Doch wenn ich nen neuen Resourcentyp einführe muss ich bestehenden Code an manchen Stellen schon noch anpassen, überall dort, wo Ressourcen ausgewertet werden etc. Ob ich da wirklich so viel "einspare".

[minastaros]

Man sollte die switch/case-Konstrukte immer so klein wie möglich halten, sonst werden sie unübersichtlich. Und natürlich immer das D.R.Y.-Prinzip beachten: Don't repeat yourself. Doppelter Code ist von Natur aus böse.

Und: ein default: gehört in jedes switch - allein schon aus Prinzip, um auch die unerwarteten Fehler abzufangen.

Ohne den genauen Sinn zu durchsteigen, meine Vorredner meinten evtl auch sowas in der Art:

int gfx_type = 0; 

switch (field)
{
    case LAKE:  gfx_type = GFX_LAKEFIELD;  break;
    case GRASS: gfx_type = GFX_GRASSFIELD; break;
    case WOOD:  gfx_type = GFX_WOODFIELD;  break;
    case SWAMP: gfx_type = GFX_SWAMPFIELD; break;
    case STONE: gfx_type = GFX_STONEFIELD; break;
    default:
      	...
}

if( 0 != gfx_type )
{
    animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, field, FALSE, ...);
    animation = &(animObject->createAnimation(FALSE, 0, FALSE, gfx_type, FALSE));
}

Übrigens ein wichtiger Unterschied zwischen Pointern und Referenzen: Referenzen soll man verwenden, ja, wenn garantiert ein Objekt dahinter steckt. Wenn es bewußt auch nicht da sein darf, dann ist ein Pointer besser.

[Reth]

minastaros schrieb:

Ohne den genauen Sinn zu durchsteigen, meine Vorredner meinten evtl auch sowas in der Art:

int gfx_type = 0; 
    
switch (field)
{
    case LAKE:  gfx_type = GFX_LAKEFIELD;  break;
    case GRASS: gfx_type = GFX_GRASSFIELD; break;
    case WOOD:  gfx_type = GFX_WOODFIELD;  break;
    case SWAMP: gfx_type = GFX_SWAMPFIELD; break;
    case STONE: gfx_type = GFX_STONEFIELD; break;
    default:
      	...
}
    
if( 0 != gfx_type )
{
    animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, field, FALSE, ...);
    animation = &(animObject->createAnimation(FALSE, 0, FALSE, gfx_type, FALSE));
}

Genau das war gemeint, zumindest hatte ich das so verstanden.

[minastaros]

Oder was in der Art:

typedef int field_t;
typedef int gfx_t;
map< field_t, gfx_t > gfxMap;

// hier gfxMap mit den GFX-Werten initialisieren

// und dann:

field_t field = rand()%5;

if( field im richtigen Bereich )
{
	animObject = AnimObjectManagerC::createAnimObject(xPos, yPos, BACKGROUND, field, FALSE, ...);
	animation = &(animObject->createAnimation(FALSE, 0, FALSE, gfxMap[ field ], FALSE));
}

[Reth]

@minastaros:

Fände ich zuviel des Guten. Wie gesagt handelt es sich ja in beiden Fällen (bei field und GFX_...) um numerische Werte (nat. Zahlen).

Allerdings frage ich mich immer noch, ob der Anzatz mit den spezielleren Animation-Klassen nicht zu viel Aufwand an allen anderen Stellen verursacht (da ich ihn aus konzeptioneller Sicht besser finde)? Wie gesagt unterscheiden sich die Animations-Objekte nur im Typ, dieser wird benötigt, wenn aus den 5 versch. Typen das Spielfeld zufällig aufgebaut wird!

[brotbernd]

Reth schrieb:

Wie gesagt unterscheiden sich die Animations-Objekte nur im Typ,

Wenn das tatsächlich so ist, dann ist eine Vererbung sinnlos. Immer wenn du aber etwas schreibst wie if animationsTyp == Diese dann MalAufsSpielfeldDieses; else if animationsTyp == Jenes dann MalAufsSpielfeldJenes; else... solltest du erwähnen jede konkrete Implementierung in eine Unterklasse irgendeiner Basisklasse, die die gemeinsame Schnittstelle definiert zu packen, aus genannten Gründen.

[Reth]

Ja, aktuell isses noch so. Allerdings werde ich im Code auch schnell merken, wie oft ich wo in solche Situationen komme. Dann wird auch refactored.

Tue mich mit dem Design (unabhängig von C++) eh immer sehr schwer und mach dazu mal nen neuen Thread auf. Ich hoffe, euch dort zu treffen und auf eure Unterstützung!

Danke nochmal!

[Reth]

So, Design-Thread ist hier: http://www.c-plusplus.net/forum/281712

Dort wird das Thema mit den GFX_...-Typen als Symbols auch nochmal relevant!