OOP ?!

[Gast]

hehejo schrieb:

@Lumpensammler
Änder mal deine [ code ] Tags in [ cpp ] (ohne Leerzeichen).
Dann bekommt dein Code nämlich ein wunderschönes Syntaxhighlighting verpasst
und wir alle können den Code leichter lesen.

Gerade gemacht !
@all:
Leute, leute..ich bin beeindruckt
Danke für die ganze Hilfe, jetzt hab ich auch mal das Gefühl auf dem richtigen Weg mit meinem c++ zu sein
<Keep going>
Lumpi~

[groovemaster]

Noch ein paar kleine Tipps:
- benutze #define nicht für Konstanten, sondern richtige, Bsp const int ende = 10;
- lass das void bei parameterlosen Funktionen weg, das ist in C++ völlig unnötig
- schau dir mal Initialisierungslisten an, und wie man diese im ctor verwendet (zB wird, sofern nichts explizit angegeben, der default ctor der Basisklasse immer aufgerufen)
- ein break nach return macht keinen Sinn
- lass das boolsche Literal bei Vergleichen weg, also einfach nur zB 'while (!m1.GetStatus() && !c1.GetStatus())'

[nicht gut]

- lass die redundanten Kommentare (wie kommt man auf die Idee eine Funktion "Ausgabe()" mit "Ausgabe" zu kommentieren?)

Glaub nicht das dein Code dadurch 'vollständiger' wird.

[camper]

wenn wir schon bei den kleinigkeiten sind
-wenn du einen default konstruktor hast, der dasselbe, wie ein konstruktor mit argument macht (nur eben mit einem ganz bestimmten wert), dann spar dir den parameterlosen konstruktor und gibt dem anderen einen default-parameter mit:
Tisch::Tisch(int anzahl = 11)

-die ganzen case fälle 2..4 könntest du leicht zusammenfassen

[Gast]

camper schrieb:

wenn wir schon bei den kleinigkeiten sind
-wenn du einen default konstruktor hast, der dasselbe, wie ein konstruktor mit argument macht (nur eben mit einem ganz bestimmten wert), dann spar dir den parameterlosen konstruktor und gibt dem anderen einen default-parameter mit:
Tisch::Tisch(int anzahl = 11)

-die ganzen case fälle 2..4 könntest du leicht zusammenfassen

Zu dem Konstruktor...11 ist der standard Wert für dieses Spiel, der benutzt werden soll wenn man das Tisch-Objekt ohne Argumente erstellt, ansonsten kann man die Münzenanzahl frei bestimmen.

[camper]

Lumpensammler schrieb:

camper schrieb:

wenn wir schon bei den kleinigkeiten sind
-wenn du einen default konstruktor hast, der dasselbe, wie ein konstruktor mit argument macht (nur eben mit einem ganz bestimmten wert), dann spar dir den parameterlosen konstruktor und gibt dem anderen einen default-parameter mit:
Tisch::Tisch(int anzahl = 11)

-die ganzen case fälle 2..4 könntest du leicht zusammenfassen

Zu dem Konstruktor...11 ist der standard Wert für dieses Spiel, der benutzt werden soll wenn man das Tisch-Objekt ohne Argumente erstellt, ansonsten kann man die Münzenanzahl frei bestimmen.

genau

[Gast]

Hab das ganze nun nochmal überarbeitet mit Hilfe eurer Tips und Vorschläge...hie der code falls ihn jemand interessiert.

//Include-Files
#include <iostream>
#include <ctime>

using namespace std;

const int ende = 10;

//Spielfeld-Klasse
class Tisch
{
private :
	int muenzenanzahl;

public :
	explicit Tisch(int anzahl = 11) //Konstruktor mit Parameter für die Münzenanzahl
	{
		muenzenanzahl = anzahl;
	}

	void Ausgabe() //Ausgabe der Anzahl der auf dem Tisch liegenden Münzen
	{
		cout << "Es liegen " << muenzenanzahl << " Muenzen auf dem Tisch" << endl;
	}

	int muenzennehmen(int anzahl)
	{
		if(anzahl > muenzenanzahl) //unmöglicher Zug
		{
			cout << "Geht nicht !" << endl;
			return 0;
		}

		else if(anzahl == muenzenanzahl)
		{
			return ende; //keine Münzen mehr übrig -> Spielende
		}

		else
		{
			muenzenanzahl-=anzahl; //Münzenanzahl verringern
			return 1;
		}
	}

	int GetMuenzenanzahl() { return muenzenanzahl; } //Münzenanzahl zurückgeben
	void SetMuenzenanzahl(int anzahl) { muenzenanzahl = anzahl; } //Münzenanzahl festlegen

};

//Mitspieler-Klasse
class Mitspieler
{
private :
	bool winner;

public :
	bool amzug;

	Mitspieler() //Standard-Konstruktor
	{
		amzug = false; //nicht am Zug
		winner = false; //noch nicht gewonnen
	}

	virtual ~Mitspieler() { } //virtueller-Destruktor
	virtual int zug(int anzahl) = 0; //virtuelle zug-Methode welche von den abgeleiteten Klassen überschrieben werden MUSS

	bool GetStatus() { return winner; } //Status zurückgeben
	void ChangeStatus() { winner = true; } //Status auf Gewinner setzen
};

//Abgeleitete Klasse für den Computergegner
class Computer : public Mitspieler
{
private :
	int r; //Zufallszahl

public :
	Computer() : Mitspieler() //Standard-Konstruktor
	{
		srand((unsigned int)time(NULL)); //Zufallsgenerator initialisieren für spätere Verwendung in der "zug"-Funktion
	}

	virtual int zug(int anzahl) //Computer-Zug
	{
		if(amzug) //nur wenn der Spieler am Zug ist
		{
			switch(anzahl)
			{
			case 4: //Sicherer Zug
				{
					cout << "Ich nehme 3" << endl;
					return 3;
				}

			case 3: //Sicherer Zug
				{
					cout << "Ich nehme 2" << endl;
					return 2;
				}

			case 2: //Sicherer Zug
				{
					cout << "Ich nehme 1" << endl;
					return 1;
				}

			case 1: //Verloren
				{
					cout << "NOOIIIIIN....ich nehme......1 <grrggrgr>" << endl;
					return 1;
				}

			default : //Default-Zug
				{
					r = rand()%3+1;
					cout << "Ich nehme " << r << endl;
					return r;
				}
			}

			amzug = false; //Zug vorbei
		}
		return 0;
	}
};

//Abgeleitete Klasse für den menschlichen Spieler
class Mensch : public Mitspieler
{
private :
	int eingabe;

public :
	Mensch() : Mitspieler() //Standard-Konstruktor
	{
		//Platzhalter
	}

	virtual int zug(int anzahl) //Menschen-Zug-Methode welche die der Basis-Klasse überschreibt
	{
		if(amzug)
		{
			do
			{
				cout << "Wieviele Muenzen wollen sie nehmen ? ";
				cin >> eingabe;
			}
			while (eingabe > 3);

			amzug = false; //Zug vorbei
			return eingabe;
		}
		else
		{
			return 0;
		}
	}
};

//Klasse welche die wichtigsten Spielfunktionen enthält
class Game
{
private:
	int eingabe;

public:
	void spiel(Mitspieler &spieler1, Mitspieler &spieler2, Tisch &tisch1) //Funktion welche das "eigentliche" Spiel enthält
	{	
		srand((unsigned int)time(NULL)); //Zufallsgenerator initialisieren, wird benötigt um zu entscheiden welcher Spieler beginnt

		int i = rand()%2;

		if(i == 1)
		{
			spieler1.amzug = true;
		}
		else
		{
			spieler2.amzug = true;
		}

		if(spieler1.amzug) //Münzenanzahl nur ausgeben wenn Spieler 1 am zug ist, ansonsten -> doppelte Ausgabe
		{
			tisch1.Ausgabe();
		}

		//Spiel-Schleife
		while (!spieler1.GetStatus() && !spieler2.GetStatus()) //so lange weiter spielen bis ein Spieler gewonnen hat
		{
			//falls "ende" zurückgegeben wird, ändern des Status vom Mitspieler auf Gewinner
			if(tisch1.muenzennehmen(spieler1.zug(tisch1.GetMuenzenanzahl())) == ende)
			{
				spieler2.ChangeStatus();
				break;
			}	

			spieler2.amzug = true; //jetzt ist Spieler2 am Zug
			tisch1.Ausgabe();

			//falls "ende" zurückgegeben wird, ändern des Status vom Mitspieler auf Gewinner
			if(tisch1.muenzennehmen(spieler2.zug(tisch1.GetMuenzenanzahl()))  == ende)
			{
				spieler1.ChangeStatus();
				break;
			}

			spieler1.amzug = true; //jetzt ist Spieler1 am Zug
			tisch1.Ausgabe();
		}
	}

	int auswahl() //Auswahls-Menü
	{
		cout << "(1) Mensch vs. Mensch" << endl;
		cout << "(2) Mensch vs. Computer" << endl;
		cout << "(3) Computer vs. Computer" << endl;
		do
		{
			cout << "Spielmodus : ";
			cin >> eingabe;
		}
		while (!cin || eingabe > 3 || eingabe < 0);

		return eingabe; //eingabe zurückgeben um darauf reagieren zu können
	}

	void auswertung(Mitspieler &spieler1, Mitspieler &spieler2) //Auswertung des Spielergebnisses
	{
		if(spieler1.GetStatus()) //falls der Status = true ist, hat Spieler 1 gewonnen
		{
			cout << "Spieler 1 hat gewonnen !" << endl;
		}
		else if (spieler2.GetStatus()) //falls der Status = true ist, hat Spieler 2 gewonnen
		{
			cout << "Spieler 2 hat gewonnen !" << endl;
		}
		else //eigentlich nicht möglich -> Fehler falls es dazu kommt
		{
			cout << "Niemand hat gewonnen ! (Hier ist doch was falsch)" << endl;
		}
	}
};

//Main-Funktion
int main()
{
	//benötigte Instanzen erstellen
	Game spiel;
	Tisch tisch1;

	Mitspieler *spieler[2]; //zwei Pointer auf Mitspieler

	//Reaktion auf die Auswahl
	switch(spiel.auswahl())
	{
	case 1:
		{
			spieler[0] = new Mensch;
			spieler[1] = new Mensch;
			break;
		}

	case 2:
		{
			spieler[0] = new Mensch;
			spieler[1] = new Computer;
			break;
		}

	case 3:
		{
			spieler[0] = new Computer;
			spieler[1] = new Computer;
			break;
		}
	}

	spiel.spiel(*spieler[0], *spieler[1], tisch1); //hier beginnt das Spiel

	spiel.auswertung(*spieler[0], *spieler[1]); //hier wird das Spiel ausgewertet

	delete spieler[0];
	delete spieler[1];

	return EXIT_SUCCESS;
}

Natürlich bin ich weiterhin offen für Ratschläge und Hinweise auf unsinnigen Kram den ich gebastelt hab.
MfG,
Lumpi~

[ness]

Lass Datenelemente in der öffentlichen Schnittstelle sein!
Globale Variablen sind nicht schöne, außerdem hast du doch Game... Hau ende da rein. So kannst du das auch besser verzahnen, z.B. indem du prüfen kannst ob mit mehr münzen als ende angefangen wird.
Neben den Ausgabefunktionen Ausgabefunktionen würde ich überladene << operatoren anbieten, die die Ausgabefunktionen benutzen (kommst du um friends drumrum). Außerdem ausgabe einen std::ostream erwarten lassen, um zum Beispiel in eine Datei zu dkumentieren. Dann vielleicht sowas wie std_Ausgabe, die Ausgabe mit cout aufruft.
Mitspieler würde ich als reines interface (nur rein Virtuelle Funktionen, keine Daten) defnieren...

[camper]

du hast den virtuellen destruktor von Mitspieler vergessen. das ist sehr böse. wenn du glück hast, schmiert das programm irgendwann ab, wenn du pech hast, wird vorher die festplatte formatiert

EDIT: zumindest dann wenn du korrekt arbeitest und alles, was du dir mit new geholt hast, auch mit delete wieder freigibst.

ich würde noch den constructor von Tisch als explizit deklarieren, sonst geht z.b. sowas:

spiel.spiel(*spieler[0], *spieler[1], 11);

falls das gewollt ist, sollte man es dokumentieren.

[ness]

Oder es werden zwielichtige Emails an deinen Arbeitgeber gesendet .

[Gast]

Hab den code nun nochmal geupdatet...kann ich das nun so abschliessen, oder ist es immer noch sehr schlimm ?
Naja...wenn ihr Verbesserungsvorschläge habt, bitte mit code, dann kann ich es leichter verstehen.
Danke an alle die hier geposted haben
MfG,
Lumpi~

[camper]

du forderst du speicher mit new an, nicht mit new[]. dann muss du ihn auch mit delete und nicht mit delete[] freigeben.

wäre noch anzumerken, dass da einiges an code-duplizität m Computespieler ist.
die case zweige 2,3,4 unterscheiden sich ja nicht wesentlich, man könnte es also einfach so zusammenfassen:

// ...
            case 4:
            case 3:
            case 2: // Sichere Züge
                {
                    cout << "Ich nehme " << anzahl - 1 << endl;
                    return anzahl - 1;
                }
//...

ähnlich könnte man bei der auswahl in main() vorgehen. gerade wenn man mit rohen zeigern umgeht, sollte man versuchen, speicheranforderung und -freigabe möglichst übersichtlich zu halten (auf exception sicherheit will ich hier nicht eingehen). hier forderst du an drei verschiedenen stellen speicher an, wenn es unübersichtlich wird, kann das schnell zu fehlern führen. es wäre also evtl. sinnvoller statt des case für jeden pointer einzeln eine if abfrage zu machen, so das sichergestellt ist, dass jeder davon genau einmal initialisiert wird. im übrigen ist die auswahl nicht sicher, denn man könnte auch 0 oder negative werte eingeben.

int spielvariante = spiel.auswahl();
    if( spielvariante == 1 || spielvariante == 2 )
    {
        spieler[0] = new Mensch;
    }
    else
    {
        spieler[0] = new Computer;
    }
    if( spielvariante == 1 )
    {
        spieler[1] = new Mensch;
    }
    else
    {
        spieler[1] = new Computer;
    }

selbst wenn auswahl unsinnige werte zurückgibt führt das in diesem falle nicht zum absturz. das ist aber z.T. auch eine frage des stils.
in diesem falle würde ich selbst es noch stärker zusammenfassen:

int spielvariante = spiel.auswahl();
    spieler[0] = ( spielvariante == 1 || spielvariante == 2 ? new Mensch : new Computer );
    spieler[1] = ( spielvariante == 1                       ? new Mensch : new Computer );

das ist aber nicht jedermanns geschmack.

[Gast]

Wo ist der Unterschied zwischen new , new[] , delete und delete[] ?

[ness]

new erstellt genu ein Objekt (erstellen=speicher bereitstellen, ctor aufrufen);
delete löscht ein solches Objekt wieder.

erstellt x objekte (Speicher für x Objekte bereitstellen, x ctors aufrufen)

delete[] löscht die Objekte wieder.
new[];delete -> es werden nicht alle Objekte gelöscht
new;delete[] -> was jetzt? Schlecht ists auf jeden fall...