C++ Frage bezüglich Speicheradressierung bei Funktionen

Thesi

Ensign
Registriert
Mai 2009
Beiträge
254
Hallo CB-Community,

ich habe mich daran gewagt C++ zu lernen und stehe noch ganz am Anfang.


Vorab: Ich weis, dass es sehr viel Text ist, aber ich hoffe dennoch, dass sich jemand findet, der sich Zeit nimmt und mir helfen kann. Vielen Dank schon mal!!
Die wichtigen Stellen im Code, sind der struct, main(), dateneingabe() und alterscheck();
Frage steht ganz unten in fett, damit sie sich vom restlichen Text abhebt.


Ich habe im Internet eine Aufgabe gefunden bei dem ein Glücksspiel programmiert werden soll, bei dem eine Zufallszahl generiert wird und dann 2 Spieler eine Zahl eingeben. Derjenige, der näher an der Zahl dran ist gewinnt.

Ich habe das ganze etwas abgewandelt. Es gibt nur einen Spieler, der um einen gewissen Einsatz gegen das Casino spielt. Somit wird nur die Zahl des Spielers über die Konsole eingegeben. Die Zahl des Casinos und die "Gewinnzahl" werden zufällig generiert.

Hiermit hatte ich auch in meiner ersten Version keine Probleme. Allerdings hatte ich dort sehr viel in der main Funktion stehen.

funktionierende Version 1:
Code:
#include <iostream>
#include <stdlib.h>
#include <string>
#include <Windows.h>


using namespace std;



//global variables

struct Spieler{
	int ialter, ikontostand;
	string svname, snname;
	char cgeschlecht;
	
};



//function prototypes
void gluecksspiel(Spieler);


int main()
{
	//local variables

	Spieler spieler;


	//Eingabe der persoenlichen Daten.		
	cout << "Geben Sie bitte Ihren Namen, Ihr Alter und Ihr Geschlecht (m/w) ein." << endl;
	cout << "Vorname: ";
	cin >> spieler.svname;
	cout << "Nachname: ";
	cin >> spieler.snname;
	cout << "Alter: ";
	cin >> spieler.ialter;
	cout << "Geschlecht: ";
	cin >> spieler.cgeschlecht;

	//Check, ob Spieler ueber 18 und geschlechtsspezifische Begruessung.
	if (spieler.ialter < 18){
		cout << "Hallo " << spieler.svname << "! Leider ist Glueckspiel erst ab 18 erlaubt." << endl;
	}
	else if (spieler.ialter >= 18 && (spieler.cgeschlecht == 'm' || spieler.cgeschlecht == 'M')){
		cout << "Guten Tag Herr " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
		cin >> spieler.ikontostand;
		cout << "\nViel Spass beim Gluecksspiel." << endl;
		gluecksspiel(spieler);
	}
	else if (spieler.ialter >= 18 && (spieler.cgeschlecht == 'w' || spieler.cgeschlecht == 'W')){
		cout << "Guten Tag Frau " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
		cin >> spieler.ikontostand;
		cout << "\nViel Spass beim Gluecksspiel." << endl;
		gluecksspiel(spieler);
	}
	
	//Manuelles Schliessen der Anwendung
	cout << "Zum Beenden der Anwendung Enter druecken...";
	cin.sync();
	cin.get();
	return 0;
}


//function definitions

void gluecksspiel(Spieler spieler){
	//local variables
	int irand, icasino, ispieler, idiffcasino, idiffspieler, ibet;
	char cweiterspielen, ceinzahlen;

	do{
		//Wetteinsatz festlegen
	bet:
		cout << "Wie viel wollen Sie setzen: ";
		cin >> ibet;
		if (ibet <= spieler.ikontostand && ibet > 0){
			spieler.ikontostand -= ibet;
		}
		else{
			cout << "Eingabe unggueltig.";
			goto bet;
		}


		//Zufallszahl und Casinozahl generieren
		irand = rand() % 100 + 1;
		icasino = rand() % 100 + 1;

		//Spielertipp (Zahl) einlesen
		cout << "Bitte geben Sie ihren Tipp zwischen 1 und 100 ein: ";
		cin >> ispieler;

		//Berechnung des "Abstandes" zwischen der Casinozahl und der Gewinnzahl 
		idiffcasino = irand - icasino;
		if (idiffcasino < 0)
			idiffcasino = idiffcasino *(-1);

		//Berechnung des "Abstandes" zwischen des Spielertipps und der Gewinnzahl 
		idiffspieler = irand - ispieler;
		if (idiffspieler < 0)
			idiffspieler = idiffspieler *(-1);

		//Ermittlung des Gewinners
		//Spieler gewinnt
		if (idiffcasino > idiffspieler){
			cout << "Sie haben gewonnen!" << endl << "Die gesuchte Zahl war " << irand << endl << "Ihr Tipp war " << ispieler << " und das Casino hat " << icasino << " getippt." << endl
				<< "Damit liegen Sie um " << idiffcasino - idiffspieler << " naeher an der richtigen Zahl." << endl;
			spieler.ikontostand += 2 * ibet;
		}
		//Casino gewinnt
		else if (idiffcasino < idiffspieler){
			cout << "Sie haben verloren!" << endl << "Die gesuchte Zahl war " << irand << endl << "Ihr Tipp war " << ispieler << " und das Casino hat " << icasino << " getippt." << endl
				<< "Damit lag das Casino um " << idiffspieler - idiffcasino << " naeher an der richtigen Zahl." << endl;
		}
		//Unentschieden
		else{
			cout << "Unentschieden" << endl << "Die gesuchte Zahl war " << irand << endl << "Ihr Tipp war " << ispieler << " und das Casino hat " << icasino << " getippt." << endl
				<< "Damit lagen Sie genauso nahe an der richtigen Zahl wie das Casino." << endl;
			spieler.ikontostand += ibet;
		}
		//Check ob noch Geld zum Spielen da ist.
		//Falls ja, kann er frei entscheiden, ob er weiterspielen will oder nicht
		if (spieler.ikontostand >0){
			
			cout << "Moechten Sie weiterspielen? (j/n)";
			cin >> cweiterspielen;
		}
		//Sonst muss er entweder erneut Geld einzahlen oder gehen.
		else{
			cout << "Sie haben kein Geld mehr. \nMoechten Sie weiteres Geld einzahlen? (j/n)" << endl;
			cin >> ceinzahlen;
			switch (ceinzahlen)
			{
			case 'J':
			case 'j':
				cout << "Wie viel moechten Sie einzahlen: ";
				cin >> spieler.ikontostand;
				break;
			case 'N':
			case 'n':
				cout << "Ohne Geld koennen Sie leider nicht weiterspielen. Auf Wiedersehen" << endl;
				break;
			}

		}
	} while (spieler.ikontostand > 0 && (cweiterspielen != 'n' && cweiterspielen != 'N'));

	cout << "Vielen Dank fuer Ihren Besuch! Auf Wiedersehen." << endl;
}


Da ich aber immer wieder lese, dass man möglichst wenig in die main Funktion schreiben soll, um seinen Code zum einen besser lesbar zu gestalten und zum anderen um einzelne Funktion besser wiederzufinden, wollte ich die Dateneingabe und den Alterscheck auslagern.

Hierbei hatte ich das Problem, dass er immer wieder in den Fall "ialter<18" gelaufen ist, auch wenn man ein Alter >18 eingegeben hat.
Hier der Code dazu:


nicht funktionierende Version 2:
Code:
#include <iostream>
#include <stdlib.h>
#include <string>
#include <Windows.h>


using namespace std;


//global variables

struct Spieler{
	int ialter, ikontostand;
	string svname, snname;
	char cgeschlecht;
};



//function prototypes
void dateneingabe(Spieler);
void alterscheck(Spieler);
void gluecksspiel(Spieler);


int main()
{
	//local variables
	Spieler spieler;

	dateneingabe(spieler);
	alterscheck(spieler);

	//Manuelles Schliessen der Anwendung
	cout << "Zum Beenden der Anwendung Enter druecken...";
	cin.sync();
	cin.get();
	return 0;
}

//function definitions

void dateneingabe(Spieler spieler)
{
	//Eingabe der persoenlichen Daten.

	cout << "Vorname: ";
	cin >> spieler.svname;
	cout << "Nachname: ";
	cin >> spieler.snname;
	cout << "Alter: ";
	cin >> spieler.ialter;
	cout << "Geschlecht: ";
	cin >> spieler.cgeschlecht;
}

void alterscheck(Spieler spieler)
{
	
	//Check, ob Spieler ueber 18 und geschlechtsspezifische Begruessung.
	if (spieler.ialter < 18){
		cout << "Hallo " << spieler.svname << "! Leider ist Glueckspiel erst ab 18 erlaubt." << endl;
	}
	else if (spieler.ialter >= 18 && (spieler.cgeschlecht == 'm' || spieler.cgeschlecht == 'M')){
		cout << "Guten Tag Herr " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
		cin >> spieler.ikontostand;
		cout << "\nViel Spass beim Gluecksspiel." << endl;
		gluecksspiel(spieler);
	}
	else if (spieler.ialter >= 18 && (spieler.cgeschlecht == 'w' || spieler.cgeschlecht == 'W')){
		cout << "Guten Tag Frau " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
		cin >> spieler.ikontostand;
		cout << "\nViel Spass beim Gluecksspiel." << endl;
		gluecksspiel(spieler);
	}
}


void gluecksspiel(Spieler spieler){
	//wie in Code 1
}


Ich habe mir dann an verschiedenen Stellen im Code den Wert und die Speicheradresse von ialter ausgeben lassen.
Hierbei bin ich dann darauf gestoßen, dass die Speicheradresse für spieler.ialter in Main eine andere ist als spieler.ialter in dateingabe().
Allerdings ist die Speicheradresse für spieler.ialter in dateneingabe() und alterscheck() die gleiche.

Meine Lösung für das Problem war, dass ich im struct Spieler für jede Variable noch einen Pointer angelegt habe und die Dateneingaben dann über die Pointer gelöst habe. Damit funktionieren die ausgelagerten Funktionen auch einwandfrei.

Funktionierende Version 3:
Code:
#include <iostream>
#include <stdlib.h>
#include <string>
#include <Windows.h>


using namespace std;


//global variables

struct Spieler{
	int ialter, ikontostand;
	string svname, snname;
	char cgeschlecht;
	int *pointeralter = &ialter, *pointerkontostand = &ikontostand;
	string *pointervname = &svname, *pointernname = &snname;
	char *pointergeschlecht = &cgeschlecht;
};



//function prototypes
void dateneingabe(Spieler);
void alterscheck(Spieler);
void gluecksspiel(Spieler);


int main()
{
	//local variables
	Spieler spieler;


	dateneingabe(spieler);
	alterscheck(spieler);


	//Manuelles Schliessen der Anwendung
	cout << "Zum Beenden der Anwendung Enter druecken...";
	cin.sync();
	cin.get();
	return 0;
}

//function definitions

void dateneingabe(Spieler spieler)
{
	//Eingabe der persoenlichen Daten.

	cout << "Geben Sie bitte Ihren Namen, Ihr Alter und Ihr Geschlecht (m/w) ein." << endl;
	cout << "Vorname: ";
	cin >> *spieler.pointervname;
	cout << "Nachname: ";
	cin >> *spieler.pointernname;
	cout << "Alter: ";
	cin >> *spieler.pointeralter;
	cout << "Geschlecht: ";
	cin >> *spieler.pointergeschlecht;
	
}

void alterscheck(Spieler spieler)
{
	
	//Check, ob Spieler ueber 18 und geschlechtsspezifische Begruessung.
	if (spieler.ialter < 18){
		cout << "Hallo " << spieler.svname << "! Leider ist Glueckspiel erst ab 18 erlaubt." << endl;
	}
	else if (spieler.ialter >= 18 && (spieler.cgeschlecht == 'm' || spieler.cgeschlecht == 'M')){
		cout << "Guten Tag Herr " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
		cin >> spieler.ikontostand;
		cout << "\nViel Spass beim Gluecksspiel." << endl;
		gluecksspiel(spieler);
	}
	else if (spieler.ialter >= 18 && (spieler.cgeschlecht == 'w' || spieler.cgeschlecht == 'W')){
		cout << "Guten Tag Frau " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
		cin >> spieler.ikontostand;
		cout << "\nViel Spass beim Gluecksspiel." << endl;
		gluecksspiel(spieler);
	}
}


void gluecksspiel(Spieler spieler){
	//wie Code 1
/



Jetzt zur eigentlichen Frage.

Kann mir jemand das mit Speicherzuweisung erklären? Ich verstehe das nicht.

Ich erzeuge doch das Objekt spieler in Main. Damit werden dann doch auch automatisch die Variablen ialter etc. für diese Spielerinstanz angelegt und ihnen eine Speicheradresse zugewiesen.
Dann übergebe ich dieses Spieler-Element an die anderen Funktionen.

Wieso haben die Variablen dann in dateneingabe() eine andere Speicheradresse?

Dies bedeutet ja, dass diese Variablen neu angelegt wurden und zwar an einer anderen Speicheradresse. Somit existiert spieler.ialter 2 Mal. :confused_alt:
Einmal in der Speicheradresse von der Main Funktion und einmal in der Speicheradresse von den externen Funktionen,
Wie kann das aber sein, wenn ich nur 1 Instanz vom Typ Spieler erzeuge?


Ich versteh es einfach nicht :(


Edit: gerade beim Absenden des Posts ist mir noch eine Idee gekommen, welche auch funktioniert.
Wenn ich alterscheck(spieler) nicht in Main aufrufe sondern in dateneingabe(), dann funktionier alles ohne Pointer.

Allerdings löst das mein Verständnisproblem mit den unterschiedlichen Speicheradressen von main und den anderen Funktionen immer noch nicht :(



Ansonsten bin ich auch offen für jeden weiteren Tipp bzgl. meines Codes, da ich ja noch ganz am Anfang meines Lernweges stehe.


Nochmal vielen Dank an diejenigen, die sich alles durchgelesen haben!!!

Gruß
thesi
 
Zuletzt bearbeitet:
Deine Werte werden aus den Funktionen nicht nach oben weitergegeben da du auf den Kopien arbeitest. Verwende statt dessen Referenzen oder gib den Wert zurück.

Ansonsten, benutze keine rohen Zeiger sondern Smart-Pointer, wie std::unique_ptr.

Viel Spaß beim C++ lernen ...
 
ich denke es geht einfach um call by value bei den funktionsaufrufen :)
das heißt die funktionen legen eine kopie der übergebenen struct an und verändern diese.

edit: wusu war leider schneller ;-) und hat auch schon eine möglichkeit genannt um das ganze zu umgehen.
 
Zuletzt bearbeitet:
Wenn du nur eine Variable übergibst, also ohne Pointer oder einer Referenz dann übergibst du eigentlich eine Kopie deiner Variable mit der deine Funktion dann iwas macht. Einfaches Beispiel:

Wenn du in der Main eine Variable mit a=3 initialisierst und einer Funktion übergibst und dann in der Funktion selber zB. schreibst
a=5 dann wird in der main a immer noch 3 bleiben.

Kannst ja den Unterschied nachschlagen
Call-by-Value und Call-by-Reference

Grüße
 
Wow, vielen Dank für diese schnelle Hilfe.

Ich habe das mit call by value und call by reference gerade nachgeschlagen.

Das werde ich jetzt so schnell sicher nicht mehr vergessen!

Mit Call by Reference brauche ich auch gar keine Pointer anlegen. So funktioniert es nun :D

Code:
//function prototypes
void dateneingabe(Spieler&);
void alterscheck(Spieler&);
...

int main()
{
	//local variables
	Spieler spieler;


	dateneingabe(spieler);
	....
}

//function definitions

void dateneingabe(Spieler &spieler)
{
	....
}

Und das das mit den Smart Pointern werde ich mir auch noch anschauen. Habe bisher auch noch gar nicht gewusst, dass es noch andere Pointer gibt, als die die ich verwendet habe...
Ja da gibt es noch sehr viel zu lernen.

Gruß
thesi
 
Willst Du noch generelles Feedback oder bist Du mit der funktionierenden Variante zufrieden?
 
Mal die ersten Dinge. Ja ist pedantisch... ist mein Job :p

Naming conventions: Gibt es wie Sand am Meer und jeder hat seine Vorlieben.
Wenn Datentyp im Variablennamen aber ein wenig leserlicher. Statt "string svname" entweder "string s_vname" (eher nicht üblich) oder "string sVname" (ungarische Notation). Ansonsten gerne ausschreiben als "string sVorname". Ansonsten finde ich ungarische Notation generell übertrieben...

Funktionsdeklarierung (Prototypen)
Bitte auch den Variablennamen hinzufügen. Macht es einfach leserlicher.
void dateneingabe(Spieler& spieler);

alterscheck()
Generell eine schlechte Funktionsbezeichnung. Denn hier wird auch eingezahlt und auch gluecksspiel() aufgerufen. Sinnvoll wäre natürlich eine einzahlen() Funktion um das zu trennen. Also die Reihenfolge der IF Verzweigungen mit den Unterfunktionen alterscheck->gluecksspiel ist nicht logisch. einzahlen() kannst Du auch in Deiner gluecksspiel() Funktion wiederverwenden. Spart also Code.
alterscheck() sollte nur Berechnen ob der Spieler "erlaubt" ist.
Zum Beispiel: bool spielerErlaubt(Spieler& spieler){return (spieler.alter >=18);}
In gluecksspiel () machst Du dann als erstes die Abfrage if(true == spielerErlaubt(spieler)){...}else{abbruch}


GOTO: NOGO!!! Einfach so tun als ob es das nicht gibt. Bitte!
Ergänzung ()

Ansonsten ist das natürlich Funktionaler Code und nicht Zeitgemäß. Das objektorientiert zu schreiben wäre hier deutlich angebrachter. Aber für den Anfang passt Funktional auch.
Ergänzung ()

Coderedundanz in alterscheck()
Natürlich auch viel doppelt wegen m/W Unterscheidung, das kann man umstrukturieren und einfacher gestalten.
Einziger Unterschied ist ja die Ausgabe von "Guten Tag Herr/Frau". Einfache Lösung (kennst Du aus Webformularen) wäre, die Anrede auch in den Spieler Struct hinzuzufügen als Feld und einzulesen. Schöner wäre aber sowas (pseudocode):
string anrede(Spieler& spieler)
string sAnrede;
switch(spieler.geschlecht) case ''w": sAnrede="Frau" / case "m": sAnrede = "Herr"
return sAnrede
----
string sAnrede = anrede(spieler);
cout << "Guten Tag " << sAnrede << " << spieler.snname << "!" << endl << "Wie viel Geld moechten Sie einzahlen: ";
cin >> spieler.ikontostand;
cout << "\nViel Spass beim Gluecksspiel." << endl;
gluecksspiel(spieler);
}

muss dann nur einmal sein^^ die Funktion anrede() kannst Du dann überall wiederverwenden wo Du diese Unterscheidung machen musst.
Ergänzung ()

Switch Case in gluecksspiel() einzahlen j/n Abfrage. Unschön und es fehlt der Default Case. Gibt jemand Z ein, was passiert?
Normal wäre dann default: cout << "Bitte mit j oder n antworten" oder so ähnlich
Ergänzung ()

smart pointer gibt es erst seit c++11 - musst Du checken ob es Dein Compiler kann (sollte) bzw es einstellen
 
Zuletzt bearbeitet:
Mal die ersten Dinge. Ja ist pedantisch... ist mein Job :p

:D

Variablennamen und Funktionsdeklaration werde ich nun ausführlicher durchführen.

Das mit alterscheck() bzw. generell mit den Funktionen bin ich aktuell dabei das besser durchzustrukturieren.
Ich will den Code auf jeden Fall nochmal überarbeiten. Prinzipiell denke ich, dass ich überall dort wo ich einen Kommentar drüber geschrieben habe, wie "Wetteinsatz festlegen" oder "Ermittlung des Gewinners" auslagern kann. Werde ich die nächsten Tage machen. Fällt mir momentan noch schwer von Anfang an zu erkennen, was ich in eine eigene Funktion packen kann und was nicht.


Magst du mir auch sagen, warum GOTO ein NOGO ist? Die Idee dahinter fand ich sehr praktisch :)

Bin bisher noch nicht weitergekommen als diese funktionale Programmierung ^^.

Bei der Coderedundanz sind wir wieder beim Auslagern.
--> Merke: Mehrfache Wiederholungen von Codefragmenten und eigenständige "Arbeiten" in eigene Funktionen.


Ja default fehlt beim switch/case --> TODO
Was daran ist unschön? Könntest du das weiter ausführen?
Ich hab switch/case zum enen gemacht, um es zu üben, und ich habe es so geschrieben, wie ich es in einigen Beispielen gesehen habe.


Danke für das Feedback!

Gruß
Thesi
 
Goto widersprechen vielen Programmierparadigmen wie strukturierter Programmierung. Zudem sind goto idr durch andere Programmierkonstrukte zu ersetzen. Zum Beispiel while schleifen. Es gibt auch Fälle wo goto Sinn macht. Aber... Oft ist die Nutzung per Codingrule im Projekt verboten. Einfach nicht angewöhnen wäre hier ein Grund.

Switch case: normal soll nach jedem case ein break stehen. Das von Dir verwendete macht ja Sinn um die Groß/Kleinschreibung zu unterscheiden. Aber hier den eingelesen wert auf Uppercase umzuwandeln und dann nur auf J oder N zu prüfen wäre weniger Code.
 
Alles klar, dann wird GOTO direkt wieder gestrichen und anders gelöst im Code --> TODO.

Ok, werde das mal probieren mit dem Umwandeln von Klein/Großschreibung --> TODO
 
Fällt mir momentan noch schwer von Anfang an zu erkennen, was ich in eine eigene Funktion packen kann und was nicht.

Wie man so eine Aufgabe in Teilbereiche zerlegt, ist natürlich ein Kreativer Prozess (oder wie bei Dir Evolutionär^^). Aber auch hier gibt es Techniken das strukturiert zu machen.

Hat man eine Prosa Beschreibung, kann man Anhand von Verben und Substantiven schon viel erkennen. Verb = Funktion. Substantiv = Objekt
Richtig gemacht hast Du es schon mit Spieler als Objekt (struct ist eigentlich auch nur eine Klasse). Und eben Dateneingabe bzw "Daten eingeben" oder alterscheck bzw Alter prüfen.

Natürlich hast Du hier kein ausfühlichen Text. Also mal Brainstorming gemacht.
Was für Akteure gibt es? Relativ einfach. Spieler und Casino.
Was kann der Spieler machen? Daten eingeben, einzahlen, setzen, aufhören, am Spiel teilnehmen
Was kann die Bank machen? Spiel ausführen (würfeln, Geld einziehen, Geld verteilen), Spieler zulassen (Alter checken, Kontostand checken)
Und schon hat man gewisse Bereiche unterteilt und Funktionen definiert die man braucht.
Ergänzung ()

Natürlich kann man das immer weiter treiben. Schauen wir uns mal das Alter an. Darf man mit 18 nicht in das Casino rein? Oder nur nicht am Spiel teilnehmen? Würdest Du als das Casino dem Türsteher vertrauen? Oder auch vor jedem Spiel den Personalausweis zeigen lassen?
Was ist jetzt wenn sich Gesetze ändern und auf einmal 21 das Limit ist. Wo muss Deine Software angepasst werden? Was ist wenn zusätzliche Kriterien hinzukommen. Sagen wir mal Ladiesday, Zutritt für m/M nicht erlaubt. Oder Dein Programm läuft Online - nun hat jedes Land ein anderes Limit oder erlaubt Glücksspiel garnicht. Wie flexibel ist Dein Programm um solche Änderungen schnell einzubauen
 
Zuletzt bearbeitet:
Del Torres schrieb:
Ergänzung vom 10.08.2016 18:42 Uhr: Ansonsten ist das natürlich Funktionaler Code und nicht Zeitgemäß. Das objektorientiert zu schreiben wäre hier deutlich angebrachter. Aber für den Anfang passt Funktional auch.

Was du meinst nennt sich prozeduale Programmierung. Funktionale Programmierung ist sehr zeitgemäß. Dagegen befindet sich Objektorientierung auf einem absteigenden Ast. Weg von iterativer Programmierung hin zu deklarativ.
 
Wozu soll übrigens der Aufruf von "cin.sync()" gut sein? Ich sehe hier keine Notwendigkeit für diesen Aufruf?
Das Deklarationen dieser Art angeht.
Code:
int ialter, ikontostand;
Das würde ich eher vermeiden. Vor allem bei komplexeren Structs und Klassen ist folgendes besser lesbar
Code:
struct Spieler{
	int ialter;
        int ikontostand;
	string svname;
        string snname;
	char cgeschlecht;
};

Außerdem am besten überhaupt nicht erst angewöhnen "using namespace" global in einer Datei zu verwenden(vor allem nicht in Header Dateien!). Falls du später einmal mehrere Bibliotheken mit verwendest oder eigene Namespaces kann dir das nämlich auf die Füße fallen. Also entweder einfach den Namespace immer davor schreiben (also z.B. std::string) oder aber local verwenden was dann so aussehen würde

Code:
void dateneingabe(Spieler spieler)
{
        using std::cin;
        using std::cout;
	//Eingabe der persoenlichen Daten. 
	cout << "Vorname: ";
	cin >> spieler.svname;
	cout << "Nachname: ";
	cin >> spieler.snname;
	cout << "Alter: ";
	cin >> spieler.ialter;
	cout << "Geschlecht: ";
	cin >> spieler.cgeschlecht;
}

Wenn du C Bibliotheken wie stdlib verwendest, dann nutze statt <stdlib.h> bitte <cstdlib>. Dort sind die Methoden dann eben auch im Namespace std enthalten.

Statt der Funktion rand() aus der stdlib solltest du am besten direkt die neue random Bibliothek nutzen welche mit C++11 eingeführt wurde.
http://www.cplusplus.com/reference/random/
http://en.cppreference.com/w/cpp/header/random
Merke soweit möglich die Nutzung alter C Bibliotheken vermeiden!

Gute Tipps findest du übrigens auf der Seite des C++ Komitees
https://isocpp.org/faq

Bzgl. Naming conventions sei noch gesagt. Bitte, bitte, bitte las die ungarische Notation sein! Ich arbeite an einem riesen Haufen Code wo Leute das anscheinend zuziehen wollten aber letzten Endes nicht bei Typänderungen auch den Prefix angepasst haben und dadurch dann so tolle Sachen wie "std::string iFilename" rauskommen.
Ich halte mich da einfach immer an die Schreibweisen der STL und der Boost Bibliothek. Bis auf Macros alles klein schreiben und statt Camel Case Schreibweise werden die Wörter mittels "_" getrennt.

Ansonsten sei noch gesagt was Del Torres schon sagte, da es auch auf mich zutrifft :D
Del Torres schrieb:
Ja ist pedantisch... ist mein Job :p
 
Zuletzt bearbeitet:
Ah ja, das mit dem Namespaces. Da hat Fonce vollkommen recht.
Das von Fonce vorgeschlagene mit "using std::cout;" etc finde ich den besten Kompromiss, persönlich ziehe ich es aber vor beim std Namespace immer std:: zu schreiben. Ist aber sicher Gewöhnungssache.

Ja, ungarische Notation ist nicht mehr gängig, aus dem genannten Problem der Datentypanpassung. In meinem Projekt haben wir Camelcase, Variablen klein, Klassen groß, Macros komplett groß mit Unterstrich und für Pointer den Prefix p. Ob jetzt Camelcase oder mit Unterstrich getrennt, schenkt sich mMn nichts und ist wirklich Geschmackssache.
 
Ja, in den meisten Fällen schreibe ich den Namespace auch aus. In einigen Fällen kann es aber die Lesbarkeit erhöhen, z.B. wenn man es mit einem längerem Namespace zu tun hat.

Dies hier ist nach meinem empfinden besser zu lesen
Code:
{
	using boost::interprocess::named_mutex;
	using boost::interprocess::scoped_lock;
	named_mutex mutex(boost::interprocess::open_or_create, filename.c_str());
	std::fstream(filename, std::ios::out);
	scoped_lock<named_mutex>(mutex);
        //more code
}
als das hier
Code:
{
	boost::interprocess::named_mutex mutex(boost::interprocess::open_or_create, filename.c_str());
	std::fstream(filename, std::ios::out);
	boost::interprocess::scoped_lock<boost::interprocess::named_mutex>(mutex);
        //mode code
}

Um mir viel Schreiberei mit langen Namespaces zu ersparten nutze ich z.B. auch folgendes

Code:
#include <boost/filesystem.hpp>

namespace fs = boost::filesystem;

//some code....

void example_function()
{
	//some code....
	fs::remove(file);
	//some code....
}
//some code....

Das wird mir z.B. nächstes Jahr die Arbeit erleichtern wenn in C++17 dann die neue filesystem Bibliothek enthalten ist welche auf boost::filesystem basiert., da ich nur "boost::filesystem" durch "std::filesystem" ersetzen muss. Man sollte dann aber natürlich drauf achten das man nicht einen Namespace nimmt der schon woanders Verwendung findet.
 
Danke für die weiteren Rückmeldungen.

Ich werde jetzt erstmal die Code überarbeiten und besser strukturieren, indem ich eure Tipps anwende, bevor ich mir neue Themen vornehme.


Wenn ich fertig bin, würdet ihr dann nochmal über den Code drüber schauen?

Gruß
thesi
 
Schaue gerne wieder drüber, geb einfach per PM kurz durch das du den neuem Code gepostest hast
 
Super, vielen Dank.

Werde ich machen :)
Ergänzung ()

Diese Random-Funktion (http://www.cplusplus.com/reference/random/) liefert bei mir leider immer dieselbe Zahl.
Für meinen Fall müsste ich doch nur die 6 auf 100 ändern.

Code:
std::default_random_engine generator;
std::uniform_int_distribution<int> distribution(1,00);
int dice_roll = distribution(generator);  // generates number in the range 1..100


Allerdings liefert er mir dann jedes Mal die Zahl 13 zurück.


Daher habe ich meine alte Random-Funktion drin gelassen.

Überarbeiteter Code

Code:
#include <iostream>
#include <string>


//global variables
struct Spieler{
	int alter;
	int kontostand;
	int bet;
	std::string vorname;
	std::string nachname;
	std::string anrede;
	char geschlecht;
	char weiterspielen;
};

//function prototypes
bool alterscheck(Spieler &spieler);
bool kontostand_check(Spieler &spieler);
void anrede(Spieler &spieler);
void begruessung(int i, Spieler &spieler);
void dateneingabe(Spieler &spieler);
void einzahlen(Spieler &spieler);
void gluecksspiel(Spieler &spieler);
void kontostand_ausgeben(Spieler &spieler);
void leerzeilen_einfuegen();
void verabschiedung();
void weiterspielen(Spieler &spieler);
void wetteinsatz(Spieler &spieler);


int main()
{
	//local variables
	Spieler spieler;

	dateneingabe(spieler);
	if (alterscheck(spieler))
	{
		einzahlen(spieler);
		gluecksspiel(spieler);
	}
	
	//Manuelles Schliessen der Anwendung
	std::cout << std::endl << "Zum Beenden der Anwendung Enter druecken...";
	std::cin.sync(); //loescht das Enter aus dem Speicher, so dass das Fenster offen bleibt. Sonst schließt sich das Fenster im Fall < 18 sofort.
	std::cin.get();
	return 0;
}

//function definitions

void dateneingabe(Spieler &spieler)
{
	//Eingabe der persoenlichen Daten.
	std::cout << "Geben Sie bitte Ihren Namen, Ihr Alter und Ihr Geschlecht (m/w) ein.\n" << std::endl;
	std::cout << "Vorname: ";
	std::cin >> spieler.vorname;
	std::cout << "Nachname: ";
	std::cin >> spieler.nachname;
	std::cout << "Alter: ";
	std::cin >> spieler.alter;
	std::cout << "Geschlecht: ";
	std::cin >> spieler.geschlecht;	
	anrede(spieler);
}

void anrede(Spieler &spieler)
{
	if (spieler.geschlecht == 'w' || spieler.geschlecht == 'W')
	{
		spieler.anrede = "Frau";
	}
	else if (spieler.geschlecht == 'm' || spieler.geschlecht == 'M')
	{
		spieler.anrede = "Mann";
	}
	else
	{
		std::cout << "Ungueltige Eingabe. Bitte geben Sie ihr geschlecht erneut ein: " << std::endl;
		std::cin >> spieler.geschlecht;
		anrede(spieler);
	}

}

bool alterscheck(Spieler &spieler)
{

	//Check, ob Spieler ueber 18 und geschlechtsspezifische Begruessung.
	if (spieler.alter < 18)
	{
		begruessung(1, spieler);
		return false;
	}
	else if (spieler.alter >= 18)
	{
		begruessung(2, spieler);
		return true;
	}
	else{
		std::cout << "Fehler bei alterscheck(), kein gueltiger Fall eingetreten" << std::endl;
		return false;
	}
}

void begruessung(int i, Spieler &spieler)
{
	switch (i)
	{
	case 1:
		std::cout << "Hallo " << spieler.vorname << "! Leider ist Glueckspiel erst ab 18 erlaubt." << std::endl;
		break;
	case 2:
		std::cout << "Guten Tag " << spieler.anrede << " " << spieler.nachname << "!" << std::endl;
		std::cout << "\nViel Spass beim Gluecksspiel." << std::endl;
		break;
	default:
		std::cout << "Fehler: default case bei begruessung()" << std::endl;
	}
}

void einzahlen(Spieler &spieler)
{
	do
	{
		std::cout << "Wie viel Geld moechten Sie einzahlen: ";
		std::cin >> spieler.kontostand;
	} while (spieler.kontostand == 0 || spieler.kontostand < 0);
}

int zufallszahl_generieren()
{
	//Zufallszahl zwischen 1 und 100 generieren
	return rand() % 100 + 1;;
}

void gewinner_ermitteln(int random, int casino_tipp, int spieler_tipp, Spieler &spieler)
{

	//local variables
	int differenz_casino;
	int differenz_spieler;
	//TODO: char oder string fuer gewonnen verloren machen und dann weitere Funktion erstellen mit Kontostand anpassen und dann mit dem char oder string aufrufen.

	//Berechnung des "Abstandes" zwischen der Casinozahl und der Gewinnzahl 
	differenz_casino = random - casino_tipp;
	if (differenz_casino < 0)
		differenz_casino = differenz_casino *(-1);

	//Berechnung des "Abstandes" zwischen des Spielertipps und der Gewinnzahl 
	differenz_spieler = random - spieler_tipp;
	if (differenz_spieler < 0)
		differenz_spieler = differenz_spieler *(-1);

	//Ermittlung des Gewinners
	//Spieler gewinnt
	if (differenz_casino > differenz_spieler)
	{
		std::cout << "Sie haben gewonnen!" << std::endl << "Die gesuchte Zahl war " << random << std::endl << "Ihr Tipp war " << spieler_tipp << " und das Casino hat " << casino_tipp << " getippt." << std::endl
			<< "Damit liegen Sie um " << differenz_casino - differenz_spieler << " naeher an der richtigen Zahl." << std::endl;
		spieler.kontostand += 2 * spieler.bet;
	}
	//Casino gewinnt
	else if (differenz_casino < differenz_spieler){
		std::cout << "Sie haben verloren!" << std::endl << "Die gesuchte Zahl war " << random << std::endl << "Ihr Tipp war " << spieler_tipp << " und das Casino hat " << casino_tipp << " getippt." << std::endl
			<< "Damit lag das Casino um " << differenz_spieler - differenz_casino << " naeher an der richtigen Zahl." << std::endl;
	}
	//Unentschieden
	else{
		std::cout << "Unentschieden" << std::endl << "Die gesuchte Zahl war " << random << std::endl << "Ihr Tipp war " << spieler_tipp << " und das Casino hat " << casino_tipp << " getippt." << std::endl
			<< "Damit lagen Sie genauso nahe an der richtigen Zahl wie das Casino." << std::endl;
		spieler.kontostand += spieler.bet;
	}

}

void kontostand_ausgeben(Spieler &spieler)
{
	std::cout << "Ihr verfuegbares Guthaben betraegt: " << spieler.kontostand << std::endl;
}

bool kontostand_check(Spieler &spieler)
{
	if (spieler.kontostand > 0)
	{
		return true;
	}
	else
	{
		return false;
	}
}

void weiterspielen(Spieler &spieler)
{
	std::cout << "Moechten Sie weiterspielen (j/n): ";
	std::cin >> spieler.weiterspielen;

	//Wenn er weiterspielen will, check ob er noch Guthaben hat. Ansonsten verabschiedung.
	if (spieler.weiterspielen == 'j' || spieler.weiterspielen == 'J')
	{
		//Guthaben > 0, dann erneut gluecksspiel starten. Sonst muss er erneut Geld einzahlen, bevor er weiterspielen kann.
		if (kontostand_check(spieler))
		{
			gluecksspiel(spieler);
		}
		else
		{
			std::cout << "Ihr Guthaben ist aufgebraucht. Um weiterspielen zu koennen, muessen Sie erneut eine Einzahlung leisten." << std::endl;
			einzahlen(spieler);
			gluecksspiel(spieler);
		}
	}
	else
	{
		verabschiedung();
	}
}

void verabschiedung()
{
	std::cout << std::endl << "Vielen Dank fuer Ihren Besuch!" << std::endl;
	std::cout << std::endl << "Hoffentlich hatten Sie viel Spass mit unserem Angebot." << std::endl;
}

void leerzeilen_einfuegen()
{
	int freie_zeilen = 5;

	for (int i = 0; i < freie_zeilen; i++)
		std::cout << std::endl;
}

void gluecksspiel(Spieler &spieler){

	//local variables
	int random;
	int casino_tipp;
	int spieler_tipp;
	
	leerzeilen_einfuegen();

	kontostand_ausgeben(spieler);

	wetteinsatz(spieler);

	//Zufallszahl und Tipp vom Casino generieren
	random = zufallszahl_generieren();
	casino_tipp = zufallszahl_generieren();

	//Spielertipp einlesen
	std::cout << "Bitte geben Sie ihren Tipp zwischen 1 und 100 ein: ";
	std::cin >> spieler_tipp;
		
	//Gewinner ermitteln
	gewinner_ermitteln(random, casino_tipp, spieler_tipp, spieler);
	
	//Abfrage, ob Spieler weiterspielen will
	weiterspielen(spieler);
}

void wetteinsatz(Spieler &spieler)
{
	//Wetteinsatz festlegen	
	std::cout << "Wie viel wollen Sie setzen: ";
	std::cin >> spieler.bet;
	if (spieler.bet <= spieler.kontostand && spieler.bet > 0){
		spieler.kontostand -= spieler.bet;
	}
	else
	{
		std::cout << std::endl << "Eingabe unggueltig." << std::endl;
		wetteinsatz(spieler);
	}
}



Gruß
thesi
 
Thesi schrieb:
Super, vielen Dank.

Werde ich machen :)
Ergänzung ()

Diese Random-Funktion (http://www.cplusplus.com/reference/random/) liefert bei mir leider immer dieselbe Zahl.
Für meinen Fall müsste ich doch nur die 6 auf 100 ändern.

Code:
std::default_random_engine generator;
std::uniform_int_distribution<int> distribution(1,00);
int dice_roll = distribution(generator);  // generates number in the range 1..100


Allerdings liefert er mir dann jedes Mal die Zahl 13 zurück.
Das liegt daran, dass die Zeile
Code:
std::default_random_engine generator;
bei jedem Funktionsaufruf eine neues lokales Engineobjekt erzeugt (was nebenbei bemerkt ziemlich kostspielig ist) und dieses jedes Mal mit dem gleichen Standardwert initialisiert. Daher bekommst du auch jedes mal das gleiche Ergebnis. Es gibt dafür verschiedenen Lösungen, aber die einfachste Variante wäre die Nutzung eines "std::random_device":

Code:
int zufallszahl_generieren()
{
	std::random_device generator;
	std::uniform_int_distribution<int> distribution(1, 100);
	return distribution(generator);  // generates number in the range 1..100
}




Thesi schrieb:
Überarbeiteter Code

Ich hätte noch ein paar Anmerkungen zu Kleinigkeiten - nichts dramatisches, aber Dinge, die man sich meiner Meinung nach erst gar nicht angewöhnen sollte:
Z.B. sieht man recht häufig folgendes Muster in deinem Code:
Code:
Typ variable;
//anderer code
initialisieren(variable);
//anderer code
nutzen(variable);
Generell sollte man aber in c++ (und den meisten anderen Programmiersprachen) Variablen erst so spät wie möglich anlegen und idealerweise auch gleich initialisieren. Das macht es i.d.R. leichter den Code zu verstehen, weil man ihren Zustand über weniger Zeilen Code nachvollziehen muss und es verhindert, dass man aus versehen auf uninitialisierte Variablen zugreift.

Z.B. übertragen auf die Initialisierung von spieler:
Code:
Spieler dateneingabe()
{
        Spieler spieler;
	//Eingabe der persoenlichen Daten.
	std::cout << "Geben Sie bitte Ihren Namen, Ihr Alter und Ihr Geschlecht (m/w) ein.\n" << std::endl;
	std::cout << "Vorname: ";
	std::cin >> spieler.vorname;
	//...
	anrede(spieler);
        return spieler;
}

int main()
{
	Spieler spieler = dateneingabe();
	if (alterscheck(spieler))
	{
		einzahlen(spieler);
		gluecksspiel(spieler);
	}
        //...
}

Der zweite Punkt, der mir aufgefallen ist, ist dass du sehr häufig std::endl verwendest. Das erzeugt aber unnötigen overhead, weil es jedes mal zu einem (relativ teuren) Buffer flush führt. In den meisten fällen fälle reicht ein simples newline "\n". Z.B. in "gewinner_ermitteln()" würde ich die Ausgabe so schreiben:
Code:
std::cout << "Sie haben gewonnen!\n"
             "Die gesuchte Zahl war " << random << "\n"
             "Ihr Tipp war " << spieler_tipp << " und das Casino hat " << casino_tipp << " getippt.\n" 
             "Damit liegen Sie um " << differenz_casino - differenz_spieler << " naeher an der richtigen Zahl." << std::endl;
std::endl braucht man eigentlich nur, wenn man sicher gehen will, dass der gesamte bisherige Text sofort auf der Konsole erscheint (bzw. in der Datei auf der Festplatte)

Dann gibt's noch ein paar Funktionen, die man etwas vereinfachen könnte:
Code:
bool alterscheck(Spieler &spieler)
{
	//Check, ob Spieler ueber 18 und geschlechtsspezifische Begruessung.
	if (spieler.alter < 18)
	{
		std::cout << "Hallo " << spieler.vorname << "! Leider ist Glueckspiel erst ab 18 erlaubt." << std::endl;
		return false;
	} 
	else
	{
		std::cout << "Guten Tag " << spieler.anrede << " " << spieler.nachname << "!\n\n"
					 "Viel Spass beim Gluecksspiel." << std::endl;
		return true;
	}
}

void einzahlen(Spieler &spieler)
{
	do
	{
		std::cout << "Wie viel Geld moechten Sie einzahlen: ";
		std::cin >> spieler.kontostand;
	} while (spieler.kontostand <= 0 );
}

bool kontostand_check(Spieler &spieler)
{
	return spieler.kontostand > 0;	
}

Außerdem vermute ich mal, dass du dich noch nicht mit Klassen beschäftigt hast. Wenn es soweit ist würde ich ein paar der Funktionen zu member funktionen von "Spieler" umschreiben.
 
Zurück
Oben