Problem mit Zufallszahlen-Prog

[_Neo_]

**Hallo allerseits!

Folgendes kleines Programm soll eigentlich dem Würfeln mit verschiedenen mehrseitigen Würfeln dienen. Leider bekomme ich jedesmal als Ergebnis "-858993456" heraus.

Kann mir jemand sagen, wo ich den Fehler drin habe? Bin C++- Neuling...**

Hauptprogramm:

#include <windows.h>
#include <iostream.h>
#include "Wuerfeln.h"

int main()
{
class Wuerfeln Zufall;
int WuerfelZahl;
int WuerfelArt; // 4, 6, 8, 10, 12, 20 oder 100
cout << "Bitte Art des Wurfels eingeben:\n"
"4 = 4-seitiger Wuerfel\n"
"6 = 6-seitiger Wuerfel\n"
"8 = 8-seitiger Wuerfel\n"
"10 = 10-seitiger Wuerfel\n"
"12 = 12-seitiger Wuerfel\n"
"20 = 20-seitiger Wuerfel\n"
"100 = 100-seitiger Wuerfel\nEingabe:\t";
cin >> WuerfelArt;
cout << "Bitte Wuerfel-Anzahl eingeben:\nEingabe:\t";
cin >> WuerfelZahl;
Zufall.Zufallswurf (WuerfelZahl, WuerfelArt);
cout << "\n" << Zufall.ZufallsWurfRueckgabe() << "\n\n\n" ;
return 0;
}

eingebundene Klasse:

// Wuerfeln.h: Schnittstelle für die Klasse Wuerfeln.

#ifndef _WUERFELN_
#define _WUERFELN_

class Wuerfeln
{
private:
int Zufallszahl;
public:
Wuerfeln();
virtual ~Wuerfeln();
Zufallswurf(int AnzWuerfel, int ArtWuerfel);
ZufallsWurfRueckgabe();
};

#endif

// Wuerfeln.cpp: Implementierung der Klasse Wuerfeln.

#include <stdlib.h>
#include "Wuerfeln.h"
#include <time.h>

Wuerfeln::Wuerfeln()
{
}

Wuerfeln::~Wuerfeln()
{
}

Wuerfeln::Zufallswurf(int AnzWuerfel, int ArtWuerfel)
{
int i;
int* Feld = new int [AnzWuerfel];

if (ArtWuerfel == 4) ArtWuerfel = 4;
else if (ArtWuerfel == 6) ArtWuerfel = 6;
else if (ArtWuerfel == ArtWuerfel = 8;
else if (ArtWuerfel == 10) ArtWuerfel = 10;
else if (ArtWuerfel == 12) ArtWuerfel = 12;
else if (ArtWuerfel == 20) ArtWuerfel = 20;
else if (ArtWuerfel == 100) ArtWuerfel = 100;
else exit(1);

for (i = 0; i <= (AnzWuerfel-1); i++)
{
static bool firstTime = true;
if (firstTime)
{
srand (time (NULL));
firstTime = false;
}

Feld[i] = rand()%ArtWuerfel+1;
Zufallszahl += Feld[i];
}
}

Wuerfeln::ZufallsWurfRueckgabe()
{
return Zufallszahl;
}

[DrGreenthumb]

Zufallszahl ist nicht initialisiert.

Mit dem "C/C++" Button kann man hier im Forum übrigens seinen Quellcode schön formatiert darstellen.

[otze]

zufallszahl ist nicht 0 initialisiert, deshalb ist ein += darauf nicht definiert.
wenn du im ctor aber sowas wie zufallszahl=0; oder einen initialisierer dafür evrwendest, wird es funktionieren.

aber ein paar punkte,die mir negativ aufgefallen sind:

#include <windows.h>
#include <iostream.h>

1. iostream.h verwendet man nichtmehr, man benutzt einfach iostream ohne h.
2. die windows.h wird hier garnicht gebraucht, wieso um himmelswillen bindest du sie dann ein?

int* Feld = new int [AnzWuerfel];

nach einem new folgt immer ein delete! nach new[] ein delete[]!
btw: wozu brauchst du ein array? die werte werden ja sofot auf zufallszahl aufaddiert, man braucht dafür kein array

#include <stdlib.h>

benutz cstdlib (auch ohne h)

if (ArtWuerfel == 4) ArtWuerfel = 4;
else if (ArtWuerfel == 6) ArtWuerfel = 6;
else if (ArtWuerfel == 8) ArtWuerfel = 8;
else if (ArtWuerfel == 10) ArtWuerfel = 10;
else if (ArtWuerfel == 12) ArtWuerfel = 12;
else if (ArtWuerfel == 20) ArtWuerfel = 20;
else if (ArtWuerfel == 100) ArtWuerfel = 100;
else exit(1);

das schießt den vogel ab. wenn artwürfel den wert 6 hat,gib ihm den wert 6.
nebenbei gesagt: die eingabe kümmert sich um die gültigkeit der eingaben, die verarbeitung will sich damit nicht lange aufhalten.
durch exit(1) wird auch jedes programm idiotensicher, denn wenn ein falscher wert eingegeben wird, wird das programm einfach gekillt, keine fehlermeldung, keine neuabfrage der daten, garnichts. super. Tolles programm ;), die wahrscheinlichkeit, dass ein DAU dieses weiterbenutzt ist nicht sehr groß, also idiotensicher

[DasPinschh]

Neo schrieb:

[b]
Wuerfeln::Zufallswurf(int AnzWuerfel, int ArtWuerfel)
{
int i;
int* Feld = new int [AnzWuerfel];

if (ArtWuerfel == 4) ArtWuerfel = 4;
else if (ArtWuerfel == 6) ArtWuerfel = 6;
else if (ArtWuerfel == ArtWuerfel = 8;
else if (ArtWuerfel == 10) ArtWuerfel = 10;
else if (ArtWuerfel == 12) ArtWuerfel = 12;
else if (ArtWuerfel == 20) ArtWuerfel = 20;
else if (ArtWuerfel == 100) ArtWuerfel = 100;
else exit(1);

for (i = 0; i <= (AnzWuerfel-1); i++)
{
static bool firstTime = true;
if (firstTime)
{
srand (time (NULL));
firstTime = false;
}

Feld[i] = rand()%ArtWuerfel+1;
Zufallszahl += Feld[i];
}
}

na das geht aber effizienter
also erstmal brauchst du deine beiden hilfsvariablen nicht, random kannste einfach einmal am anfang initialisieren, ...

Wuerfeln::Zufallswurf(int AnzWuerfel, int ArtWuerfel)
{
	if (!(ArtWuerfel == 4 ||
              ArtWuerfel == 6 ||
              ArtWuerfel == 8 ||
              ArtWuerfel == 10 ||
              ArtWuerfel == 12 ||
              ArtWuerfel == 20 ||
              ArtWuerfel == 100))	
                 exit(1); // ist schlecht, besser ist es einen Fehlerwert zurückzugeben
         srand (time (NULL)); 
	for (int i = 0; i <= (AnzWuerfel-1); i++)
	    Zufallszahl += rand()%ArtWuerfel+1;
}

aso ja, warum ein virtueller destruktor?

[otze]

aso ja, warum ein virtueller destruktor?

damit man auch ein würfelergebnis von runden würfeln abfragen kann?
ehrlich gesagt, dass hab ich mich auch gefragt, aber da es kein offensichtlicher fehler ist, sondern nur schlechtes design,hab ichs mal dabei belassen.

die Klasse hat selber keinen echten wert, der als basisklasse für irgendwas anderes dient. Ich kenn nichts was ein "würfeln" ist, bei Klassen, die aktionen darstellen tue ich mich in punkto vererbung schon sehr schwer,was mit ein grund dafür ist, wieso ich die Klasse ansich kritisieren würde,wenn, dann eine Klasse "Wuerfel",die als aktion "wuerfeln" hat, im ctor die seiten übermittelt bekommt. diesen würfel könnte man prima vererben, zb an einen "Würfel", der zb ne trapez form hat, was eine verschiebung der wahrscheinlichkeiten zufolge hätte,oder ein würfel,der wirklich rund ist.

[DEvent]

ich mach mich zwar selben über nubs lustig
aber das

if (ArtWuerfel == 4) ArtWuerfel = 4;
else if (ArtWuerfel == 6) ArtWuerfel = 6;
else if (ArtWuerfel == ArtWuerfel = 8;
else if (ArtWuerfel == 10) ArtWuerfel = 10;
else if (ArtWuerfel == 12) ArtWuerfel = 12;
else if (ArtWuerfel == 20) ArtWuerfel = 20;
else if (ArtWuerfel == 100) ArtWuerfel = 100;
else exit(1);

ist absolut genial

achja, falls es noch keiner gesagt hat:
int* Feld = new int [AnzWuerfel];
wird nicht freigegeben
und die art des würfels hast du doch schon angegeben
int ArtWuerfel)
das ist doch die anzahl der seiten des würfels
wenn du einen runden würfel haben willst kannste nur ArtWuerfel > 1000000 oder so
und noch ein tipp:

class Wurfel
{
    public:
        Wurfel(int typ) : derTyp(typ)
        {
            srand(time(0));
        }
        int operator()()
        {
            return rand()%derTyp+1;
        }
    private:
        int derTyp;
}

// Aufruf:
{
    ...
    Wurfel wurfel = Wurfel(100);
    int zufall = wurfel();
}

[_Neo_]

Erstmal danke für Eure schnelle Hilfe!
Hab gerade gemerkt, dass ich an einigen Stellen mehr als krum gedacht habe 3 Bücher lesen ohne Übung ist halt nicht alles^^. Ich fange gerade erst an, das Erlesene zu üben (daher diesen Kleinkram hier auch als Klasse erprobt).
(windows.h hatte ich ausversehen drin gelassen, habe zuerst versucht die ProgAusgabe über Windows-Fenster zu erzeugen)

Eure Kritiken habe ich soweit verstanden und nach meinem bisherigen Wissen umgesetzt:
So besser?

// main.cpp

#include <iostream.h>
#include "Wuerfeln.h"

int main()
{
	class Wuerfeln Zufall;
	int WuerfelZahl;
	int WuerfelArt; // 4, 6, 8, 10, 12, 20 oder 100
	cout << "Bitte Art des Wurfels eingeben:\n"
		"4 = 4-seitiger Wuerfel\n"
		"6 = 6-seitiger Wuerfel\n"
		"8 = 8-seitiger Wuerfel\n"
		"10 = 10-seitiger Wuerfel\n"
		"12 = 12-seitiger Wuerfel\n"
		"20 = 20-seitiger Wuerfel\n"
		"100 = 100-seitiger Wuerfel\nEingabe:\t";
	cin >> WuerfelArt;
	cout << "Bitte Wuerfel-Anzahl eingeben:\nEingabe:\t";
	cin >> WuerfelZahl;
	Zufall.Zufallswurf (WuerfelZahl, WuerfelArt);
	cout << "\n" << Zufall.ZufallsWurfRueckgabe() << "\n\n\n" ;
	return 0;
}

// Wuerfeln.h

#ifndef _WUERFELN_
#define _WUERFELN_

class Wuerfeln  
{
private:
	int Zufallszahl;
public:
	Wuerfeln();
	~Wuerfeln();
	Zufallswurf(int AnzWuerfel, int ArtWuerfel);
	ZufallsWurfRueckgabe();
};

#endif

// Wuerfeln.cpp

#include <stdlib.h>
#include "Wuerfeln.h"
#include <time.h>

Wuerfeln::Wuerfeln()
{
}

Wuerfeln::~Wuerfeln()
{
}

Wuerfeln::Zufallswurf(int AnzWuerfel, int ArtWuerfel)
{
	Zufallszahl = 0;

	if (!(ArtWuerfel == 4 ||
              ArtWuerfel == 6 ||
              ArtWuerfel == 8 ||
              ArtWuerfel == 10 ||
              ArtWuerfel == 12 ||
              ArtWuerfel == 20 ||
              ArtWuerfel == 100))    
    exit(1);
    srand (time (NULL)); 
    for (int i = 0; i <= (AnzWuerfel-1); i++)
        Zufallszahl += rand()%ArtWuerfel+1;
	}

Wuerfeln::ZufallsWurfRueckgabe()
{
	return Zufallszahl;
}

[_Neo_]

Die Zufallszahlen werden jetzt übrigens tatsächlich richtig ausgegeben (keine Minuszahl mehr), lag also daran, dass ich Zufallzahl nicht genullt hatte.

[otze]

fast: die eingabe sollte sich nun nurnoch darum kümmern, dass sie korrekt ist, dh der teil

if (!(ArtWuerfel == 4 ||
              ArtWuerfel == 6 ||
              ArtWuerfel == 8 ||
              ArtWuerfel == 10 ||
              ArtWuerfel == 12 ||
              ArtWuerfel == 20 ||
              ArtWuerfel == 100))    
    exit(1);

sollte in die main, und da du nun die möglichkeit hast, auf die falsche eingabe zu reagieren, sollte auch eine neuabfrage der daten erfolgen, wenn die zuletzt eingegebenen daten nicht stimmen.

[_Neo_]

Habe aus der Klassen-CPP nun die if- Anweisungen rausgenommen und folgende Ergänzungen an der richtigen Stelle in die main.cpp eingefügt.
Nun müsste das passen, klappt zumindest gut.

#include <stdlib.h>

if (WuerfelArt == 0) exit(1);
else if (!(WuerfelArt == 4 || WuerfelArt == 6 || WuerfelArt == 8 ||
	WuerfelArt == 10 || WuerfelArt == 12 || WuerfelArt == 20 ||
	WuerfelArt == 100)) main();

[otze]

oh mein gott...nene, so geht das nicht gut, bzw nicht in größeren projekten.

int main()
{
    class Wuerfeln Zufall;
    int WuerfelZahl;
    int WuerfelArt; // 4, 6, 8, 10, 12, 20 oder 100
    cout << "Bitte Art des Wurfels eingeben:\n"
        "4 = 4-seitiger Wuerfel\n"
        "6 = 6-seitiger Wuerfel\n"
        "8 = 8-seitiger Wuerfel\n"
        "10 = 10-seitiger Wuerfel\n"
        "12 = 12-seitiger Wuerfel\n"
        "20 = 20-seitiger Wuerfel\n"
        "100 = 100-seitiger Wuerfel\nEingabe:\t";
        cin >> WuerfelArt;
        while(!(WuerfelArt == 4 || WuerfelArt == 6 || WuerfelArt == 8 || WuerfelArt == 10 || WuerfelArt == 12 || WuerfelArt == 20 || WuerfelArt == 100)){
            cout<<"bitte geben sie einen gültigen wert für die Wuerfelart an"<<endl;
            cin>>WuerfelArt;
        }
    cout << "Bitte Wuerfel-Anzahl eingeben:\nEingabe:\t";
    cin >> WuerfelZahl;
    Zufall.Zufallswurf (WuerfelZahl, WuerfelArt);
    cout << "\n" << Zufall.ZufallsWurfRueckgabe() << "\n\n\n" ;
    return 0;
}

dabei wird die eingabe solange wiederholt, bis eine gültige eingabe zustande kommt, ohne die main andauerd rekursiv aufzurufen, was einige probleme nach sich ziehen kann.

achja: versuch keinen gebrauch von exit zu machen, exit ist nicht nur unschön, sodnern macht auch den code nicht sonderlich viel lesbarer.

[_Neo_]

Ich hätte noch dazuposten sollen, dass ich im Eingabemenü, die Möglichkeit angeboten habe, per Eingabe '0', das Prog zu beenden. Somit hat der User die einfache Möglichkeit, das Prog per '0' zu verlassen.
Aber ich gebe Dir Recht, Deine Variante, sieht weitaus effektiver aus.