Wahrscheinlichkeits-Rechner [Würfel] (Konsolenanwendung)

[Moorky Dragonstrike]

Hey Leute,

das ist jetzt mein 4. Thema und in den letzten 3 habe ich immer wieder gelesen wie miserabel mein Quelltext in C++ ist.

Ich habe nun einen Wahrscheinlichkeits-Rechner in C++ programmiert und mir wirklich mühe gegeben damit der Quelltext sauber und ordentlich aussieht.

Könnt ihr nur mal ein Blick über den Quelltext werfen oder das Programm ausprobieren und mir sagen wie ihr es findet und was ich schöner schreiben könnte?
(Tut mir leid das ich keine Kommentare eingefügt habe aber darauf verzichte ich meist in solch kleinen Projekten. (unter 500 Zeilen.))

Hier der Quelltext:

#include <iostream>
#include <ctime>
#include <conio.h>
#include <Windows.h>

using namespace std;

void WSR_W_S ();

void WSR_W_L ();

int main ()
{
	int Phase = 0;
	int Key = 0;

	while (true)
	{
		cout << "Wahrscheinlichkeits-Rechner (W\201rfel) Version: 1.0 - By M.M.B.\n\n\n";

		if (Phase == 0)
		{
			cout << "[1] Wahrscheinlichkeits-Rechner (W\201rfel) [Schnell]\n\n";
			cout << "[2] Wahrscheinlichkeits-Rechner (W\201rfel) [Langsam]\n\n";
			cout << "[Esc] EXIT\n\n";

			Key = getch();

			if (Key == 49 || Key == 33)
				Phase = 1;

			if (Key == 50 || Key == 34)
				Phase = 2;

			if (Key == 27)
				Phase = 3;
		}

		else if (Phase == 1)
		{
			WSR_W_S ();
			Phase = 0;
		}

		else if (Phase == 2)
		{
			WSR_W_L ();
			Phase = 0;
		}

		else if (Phase == 3)
			return 0;

		system("CLS");
	}

	cout << "\n\n\n";
	system("PAUSE");
	return 0;
}

void WSR_W_S ()
{
	srand(time(0));

	unsigned max = 0;
	int x = 0;

	int wTest = 0;

	while (wTest != 1)
	{
		cout << "Seiten des W\201rfels: ";
		cin >> x;

		if (x > 0 && x <= 100)
		{
			cout << "\n\nRunden: ";
			cin >> max;

			if (max > 0)
				wTest = 1;

			else
				cout << "\n\n     ![Die Werte liegen nicht im G\201ltigkeitsbereich]!\n\n\n";
		}

		else
			cout << "\n\n     ![Die Werte liegen nicht im G\201ltigkeitsbereich]!\n\n\n";
	}

    int *counter = new int[x];

	for (int i = 0; i < x; i++)
		counter[i] = 0;

	system("CLS");

	cout << "Wahrscheinlichkeits-Rechner (W\201rfel) Version: 1.0 - By M.M.B.\n\n\n";

	cout << "Wahrscheinlichkeit wird berechnet...";

    for (unsigned i = 0; i < max; ++i)
    {
        ++counter[rand() % x];
    }

	system("CLS");

	cout << "Wahrscheinlichkeits-Rechner (W\201rfel) Version: 1.0 - By M.M.B.\n\n\n";

    cout << "Runden: " << max << "\n\n";

    for (unsigned i = 0; i < x; ++i)
      cout << "Seite " << i + 1 << ": " << counter[i] << " = " << 1.*counter[i]/max*100 << "%\n";

	delete[] counter;

    cout << "\n\n";
    system("PAUSE");
};

void WSR_W_L ()
{
	srand(time(0));

	int Key = 0;
	int wEsc = 0;
	int x = 0;
	int Zaehler = 0;
	int Geschwindigkeit = 1;

	cout << "Seiten des W\201rfels: ";
	cin >> x;

	cout << "\n\nGeschwindigkeit der Runden (in ms): ";
	cin >> Geschwindigkeit;

	int *counter = new int[x];

	for (int i = 0; i < x; i++)
		counter[i] = 0;

	system("CLS");

	while (wEsc != 1)
	{
		cout << "Wahrscheinlichkeits-Rechner (W\201rfel) Version: 1.0 - By M.M.B.\n\n\n";

		cout << "Runde: " << Zaehler << "\n\n";

		for (unsigned i = 0; i < x; ++i)
          cout << "Seite " << i + 1 << ": " << counter[i] << " = " << 1.*counter[i]/Zaehler*100 << "%\n";

		if (kbhit())
			Key = getch();

		if (Key == 27)
			wEsc = 1;

		++counter[rand() % x];
		Zaehler++;

		Sleep(Geschwindigkeit);

		system("CLS");
	}

	delete[] counter;

};

[SeppJ]

Ich muss leider sagen, das ist immer noch recht schlecht.

Brainstorming zu Verbesserugsmöglichkeiten, nur durch schnelles überfliegen des Codes:
- Aussagekräftigere Bezeichner benutzen! WSR_W_S? WSR_W_L?
- kein new für Arrays benutzen (und zu 99.9% auch sonst nicht), vector nehmen
- Wenn du '1' meinst, dann schreib auch '1', nicht 49
- Sauberen Kontrollfluss benutzen. In deinem Code gibt es Zeilen, die niemals erreicht werden können
- srand korrekt benutzen
- Variablen dort definieren, wo sie gebraucht werden, nicht am Funktionsanfang
- Wenn sich gleicher Code oft wiederholt, ist das schlecht. Beispiel: Wie viele Stellen im Code müsstest du ändern, wenn du zu Version 1.1 gehst? Im Idealfall gibt es keine zwei Zeilen im Code, die sich auch nur ähnlich sehen.
- Das ganze ist sehr prozedural, du nutzt an keiner Stelle dir Möglichkeiten, die C++ bietet. Es ist C mit cout.
- srand, rand und system sind in cstdio deklariert. Fehlt.
- Wenn dein Compiler nicht korrekt mit Umlauten umgehen kann, dann würde ich ja wenigstens so pedantisch sein und die korrekten UCS-Literale benutzen. Ein 'ü' wäre da '\U00FC'.

[Moorky Dragonstrike]

SeppJ schrieb:

Ich muss leider sagen, das ist immer noch recht schlecht.

Brainstorming zu Verbesserugsmöglichkeiten, nur durch schnelles überfliegen des Codes:
- Aussagekräftigere Bezeichner benutzen! WSR_W_S? WSR_W_L?
- kein new für Arrays benutzen (und zu 99.9% auch sonst nicht), vector nehmen
- Wenn du '1' meinst, dann schreib auch '1', nicht 49
- Sauberen Kontrollfluss benutzen. In deinem Code gibt es Zeilen, die niemals erreicht werden können
- srand korrekt benutzen
- Variablen dort definieren, wo sie gebraucht werden, nicht am Funktionsanfang
- Wenn sich gleicher Code oft wiederholt, ist das schlecht. Beispiel: Wie viele Stellen im Code müsstest du ändern, wenn du zu Version 1.1 gehst? Im Idealfall gibt es keine zwei Zeilen im Code, die sich auch nur ähnlich sehen.
- Das ganze ist sehr prozedural, du nutzt an keiner Stelle dir Möglichkeiten, die C++ bietet. Es ist C mit cout.
- srand, rand und system sind in cstdio deklariert. Fehlt.
- Wenn dein Compiler nicht korrekt mit Umlauten umgehen kann, dann würde ich ja wenigstens so pedantisch sein und die korrekten UCS-Literale benutzen. Ein 'ü' wäre da '\U00FC'.

Danke für die Antwort
Leider kenne ich nur cout und habe mich nochnicht mit print beschäftigt.
Genauso wie "vector" das habe ich nochnie benutzt und kenne das garnicht..
Ein paar Fragen habe ich aber:
Was meinst du mit: "- Wenn du '1' meinst, dann schreib auch '1', nicht 49"
Und: "- srand korrekt benutzen"
Und: Wieso muss ich srand, rand und system in cstdio deklarieren? Es klappt ja auch so.

[SeppJ]

Moorky Dragonstrike schrieb:

Danke für die Antwort
Leider kenne ich nur cout und habe mich nochnicht mit print beschäftigt.

Da hast du mich vollkommen missverstanden. Ich meinte: Dein Programm sieht so aus wie ein (anfängerhaftes) C-Programm, an keiner Stelle nutzt du die Möglichkeiten von C++. Würdest du statt iostreams die cstdio benutzen so wäre es waschechtes C. So ist das einzige, was hier einen C++-Compiler benötigt eben das cout. Daher: C mit cout.

Genauso wie "vector" das habe ich nochnie benutzt und kenne das garnicht..

Wie lernst du C++? Wie kannst du new kennen, aber nicht vector?

Ein paar Fragen habe ich aber:
Was meinst du mit: "- Wenn du '1' meinst, dann schreib auch '1', nicht 49"

Was soll denn 49 sein? Du meinst doch das Zeichen '1'. Wieso schreibst du dann nicht '1'?

Und: "- srand korrekt benutzen"

Was macht denn srand? Es initialisiert den (globalen) Zufallszahlengenerator. Wieso machst du das mehrmals im Programm?

Und: Wieso muss ich srand, rand und system in cstdio deklarieren? Es klappt ja auch so.

Weil's falsch ist, wenn du es nicht machst. Das etwas in einer bestimmten Implementierung zufällig doch klappt, macht es nicht richtig. Bei der nächsten Version des Compilers oder bei einem ganz anderen System funktioniert es dann nicht.

[Moorky Dragonstrike]

1. Ich lerne C++ mit einem Buch "C++ für Spieleprogrammierer" an dem Punkt wo ich bin gibt es noch kein "Vector".

2. Ich muss dort 49 eingeben weil das die ID der Taste ist, wenn ich die Taste 1 drücke übergibt es dem "KEY" durch "getch();" den Wert "49".

3. Ich dachte man muss srand in jeder Funktion neu bestimmen

4.

Dein Programm sieht so aus wie ein (anfängerhaftes) C-Programm, an keiner Stelle nutzt du die Möglichkeiten von C++.

Was habe ich denn noch für Möglichkeiten? Ich dachte schon das ich C++ -haftig programmiere, das Buch womit ich lerne handelt vom Anfänger-Dasein mit einfachen Befehlen wir "cout" bis zum ersten 2D Spiel mit 2D Grafik, das Buch jedoch habe ich erst zu 2/3 durchgenommen.

[SeppJ]

Moorky Dragonstrike schrieb:

2. Ich muss dort 49 eingeben weil das die ID der Taste ist, wenn ich die Taste 1 drücke übergibt es dem "KEY" durch "getch();" den Wert "49".

Nein, er übergibt dir den Wert '1' (das Zeichen!), was möglicherweise auch irgendwie dem Zahlenwert 49 entspricht. Dir ist bewusst, das Zeichen und Zahlen im Computer ein und dasselbe sind? Es gibt irgendwelche Tabellen, in denen steht, welches Zeichen welcher Zahl entspricht. Aber wer will schon dauernd so etwas nachschlagen? Wenn du also das Zeichen '1' meinst, dann schreib auch '1'.

4.

Dein Programm sieht so aus wie ein (anfängerhaftes) C-Programm, an keiner Stelle nutzt du die Möglichkeiten von C++.

Was habe ich denn noch für Möglichkeiten?

Wie soll man das erklären? Du scheinst keine der Möglichkeiten zu kennen, es ist schwer jemandem kurz zu erklären, was er nicht weiß. Schau mal vielleicht hier:
http://www.c-plusplus.net/forum/322028

1. Ich lerne C++ mit einem Buch "C++ für Spieleprogrammierer" an dem Punkt wo ich bin gibt es noch kein "Vector".

Vermutlich liegt hier die Ursache. Spieleprogrammierer benutzen erfahrungsgemäß selten modernen Stil und Bücher mit solchen Titeln klingen höchst verdächtig. In meiner Signatur sind Threads mit Buchtipps verlinkt.

[Moorky Dragonstrike]

Nein, er übergibt dir den Wert '1' (das Zeichen!), was möglicherweise auch irgendwie dem Zahlenwert 49 entspricht. Dir ist bewusst, das Zeichen und Zahlen im Computer ein und dasselbe sind? Es gibt irgendwelche Tabellen, in denen steht, welches Zeichen welcher Zahl entspricht. Aber wer will schon dauernd so etwas nachschlagen? Wenn du also das Zeichen '1' meinst, dann schreib auch '1'.

Versteh ich nicht... führe mal dieses Programm aus:

#include <iostream>
#include <conio.h>

using namespace std;

int main ()
{
	int Key = 0;

	while (true)
	{
		cout << Key;
		Key = getch();
		system("CLS");
	}

	cout << "\n\n\n";
	system("PAUSE");
	return 0;
}

Da wird bei der Taste [1] der Wert "49" übergeben...
(Das Programm sagt dir den Wert der bei einer Taste übergeben wird daher benutzte ich z.B. für [1] auch immer "49")

[SeppJ]

Guck mal:
http://ideone.com/qzcN7p
Vollzieh mal bei jeder Zeile nach, was warum ausgegeben wird und was das für das Verhältnis von Zahlen und Zeichen bedeutet.

[Moorky Dragonstrike]

SeppJ schrieb:

Guck mal:
http://ideone.com/qzcN7p
Vollzieh mal bei jeder Zeile nach, was warum ausgegeben wird und was das für das Verhältnis von Zahlen und Zeichen bedeutet.

Ah okay! Danke!