Erstes Programm mit Klasse - sinnvoll?

Hallo zusammen!

Ich habe mein erstes C++-Programm geschrieben, das über "Hello World!" hinausgeht und sogar eine Klasse mit Funktionen enthält.

Falls jemand von euch Zeit hätte, würde ich mich sehr freuen, wenn er es sich ansähe und mir Rückmeldung gäbe, ob ich irgendwelche Fehler gemacht habe, die ich mir schnell abgewöhnen sollte.

Inhaltlich scheint es zu funktionieren. Das Programm "wirft" mit Darts auf ein Einheitsquadrat [0,1] x [0,1] und zählt die Anzahl der Treffer im Einheitskreis. Daraus wird eine Näherung für Pi berechnet.

#include <iostream>
#include <math.h>
#include <time.h>
#include <stdlib.h>

class dart{
	private:
	double x,y;			//Koordinaten

	public:
	dart(){};			//Kontruktor leer
	~dart(){};			//Destruktor leer
	void werfen(void);		//Koordinaten zufaellig bestimmen
	bool getroffen(void);		//Falls im Einheitskreis
};

void dart::werfen(void){
	x = (rand() % 1000) / 1000.0;	//Zahl zwischen 0 und 1 mit 3 Nachkommastellen
	y = (rand() % 1000) / 1000.0;	//Hier auch
}

bool dart::getroffen(){
	if(pow(x,2)+pow(y,2)<=1) return true;	//Wenn x^2+y^2<=1 ist, dann im Kreis
	else return false;
}

int main(void){
	srand(time(NULL));			//Zur Initialisierung von rand()

	int wuerfe, treffer=0;
	double pi;				//Schaetzung fuer Pi
	dart D;					//Wird gleich oft geworfen

	std::cout << "Werfe Darts auf Quadrat [0,1] X [0,1]. Wie viele soll ich werfen? ";
	std::cin >> wuerfe;			//Anzahl der Wurfe abfragen

	for(int i=1; i<=wuerfe; i++){
	D.werfen();				//erst werfen...
	if(D.getroffen()) treffer++;		//... und dann sehen, ob ich getroffen habe
	}

	pi = 4.0*treffer/wuerfe;		//mal 4 wegen Wurf auf Viertelkreis

	std::cout << "Habe "<< wuerfe << " mal geworfen und " << treffer << " mal die Einheitsscheibe getroffen." << std::endl;
	std::cout << "Meine Schaetzung fuer Pi ist " << pi << "." << std::endl;
}

Gruß
alaundo

- kommentiere nicht das offensichtliche
- keine leeren Konstruktoren / Destruktoren
- verwende cmath, cstdlib, ctime statt der alten C-Header

Columbo

Du musst Default-Konstruktoren oder -Destruktoren die überhaupt nichts tun hier nicht selber angeben.
pow(y,2) solltest du schleunigst durch y*y ersetzen. Das dürfte deutlich schneller sein.
"Zahl zwischen 0 und 1 mit 3 Nachkommastellen" macht so keinen Sinn. Eine Fließkommazahl speichert ihr Ergebnis nicht in dezimaler Schreibweise sondern rundet sie (höchstwahrscheinlich).
Du solltest stattdessen rand() durch RAND_MAX teilen.
void in leeren Parameterlisten ist in C++ nicht nötig.
<stdlib.h> ist der falsche Header - du musst das C++-äquivalent einbinden, <cstdlib> . (Das .h am Ende wird durch ein c am Anfang ersetzt)
Deklariere Variablen dort wo du sie brauchst, nicht am Anfang der main ! Das gilt hier bspw. für pi .

Jockelx

Hallo,

ziemlich gut für ein erstes nicht-Hallo-Welt-Programm.
Konstruktor & Destruktor solltest du weglassen. Das irritiert nur, wenn die expliziet angelegt werden und nix machen.

void als Parameter genauso weglassen oder es zumindest einheitlich machen.
Den Block der for-Schleife solltest du einrücken.
Variablen erst da deklarieren, wo Sie gebraucht werden (pi in main).

Wow, vielen Dank für die schnellen Antworten!

Habe die voids weggelassen, einige Kommentare reduziert, die richtigen Header eingebunden, Kontruktor und Destruktor entfernt. Ist alles klar für mich.

RAND_MAX ist natürlich eine gute Idee, habe ich jetzt so implementiert (und direkt den Fehler gemacht, nicht zu bedenken, dass es eine Ganzzahl ist...).

Die Einrückung habe ich jetzt auch gemacht und meine Variablen da deklariert, wo ich sie brauche.

pow(y,2) solltest du schleunigst durch y*y ersetzen. Das dürfte deutlich schneller sein.

Tatsächlich, aber warum? Ich habe bisher eigentlich immer die Funktionen aus math benutzt, weil ich mir gedacht habe, dass ich so oft wie möglich auf bereits vorhandene Funktionen zurückgreifen sollte. Gibt es da Richtlinien, wann man das machen sollte?

Ich hänge auch nochmal den aktualisierten Code an:

#include <iostream>
#include <ctime>
#include <cstdlib>

class dart{
	private:
	double x,y;			

	public:			
	void werfen();		//Koordinaten zufaellig bestimmen
	bool getroffen();		//Falls im Einheitskreis
};

void dart::werfen(){
	x = 1.0 * rand() / RAND_MAX;
	y = 1.0 * rand() / RAND_MAX;	
}

bool dart::getroffen(){
	if(x*x+y*y<=1) return true;	//Wenn x^2+y^2<=1 ist, dann im Kreis
	else return false;
}

int main(){
	srand(time(NULL));			//Zur Initialisierung von rand()

	int wuerfe;				

	std::cout << "Werfe Darts auf Quadrat [0,1] X [0,1]. Wie viele soll ich werfen? ";
	std::cin >> wuerfe;			//Anzahl der Wurfe abfragen

	dart D;
	int treffer=0;
	for(int i=1; i<=wuerfe; i++){
		D.werfen();				
		if(D.getroffen()) treffer++;		
	}

	double pi = 4.0*treffer/wuerfe;		//mal 4 wegen Wurf auf Viertelkreis

	std::cout << "Habe "<< wuerfe << " mal geworfen und " << treffer << " mal die Einheitsscheibe getroffen." << std::endl;
	std::cout << "Meine Schaetzung fuer Pi ist " << pi << "." << std::endl;
}

Danke!
alaundo

Jockelx

Das pow war ja nicht total falsch oder so, aber es ist halt ein bisschen 'mit Kanonen auf Spatzen schießen'.
Die allgemeine Funktion muss mit belieber Basis & Exponenten zurecht kommen und wird sicher nichts besseres als x*x machen.
Was denke ich auch nicht schlechter lesbar ist.

Nebenbei:
Deine for-Schleife (i=1; i <= n) ist natürlich auch nicht falsch, aber ich würde mir direkt angewöhnen von 0 zu zählen (i=0; i<n), so wie du es später mal bei Arrays & co auch machen musst.

Skym0sh0

x*x ist für den Computer nur mal eben schnell eine Multiplikation, die fast keine Zeit braucht.

std::pow ist auf den generellen Fall ausgelegt, sprich pow kann auch mit krummen Parametern rechnen. Das macht es flexibel, aber für so kleine Kinkerlitzen eben langsamer. Die Faustregel ist x² händisch auszuschreiben und alles andere mit pow machen. Ausnahme bilden Multiplikationen un Divisionen mit/durch Zweierpotenzen, das geht sehr schnell mit << und >>.

(x*x*x wird öfters vielleicht auch noch schneller sein als pow(x, 3), vermutlich)

Kleinigkeit noch:

if(x*x+y*y<=1) return true;	//Wenn x^2+y^2<=1 ist, dann im Kreis
	else return false;

// geht auch einfacher mit
return x*x+y*y<=1;

Columbo

Die Faustregel ist x² händisch auszuschreiben und alles andere mit pow machen.

Wenn der Exponent zur Compile-Zeit feststeht, dann sollte man auf jeden Fall boost::math::pow verwenden oder es nachschreiben. Das wird definitiv einen Performance-Boost gegenüber pow bringen.

(x*x*x wird öfters vielleicht auch noch schneller sein als pow(x, 3), vermutlich)

Nicht vermutlich sondern hundertprozentig. Ob es viel schneller ist, ist natürlich eine andere Frage.

camper

alaundo schrieb:

double pi = 4.0*treffer/wuerfe;		//mal 4 wegen Wurf auf Viertelkreis

Mit diesem Kommentar stimmt etwas nicht. Wäre der Faktor wirklich ein anderer, wenn die Würfe auf den Vollkreis oder z.B. nur auf einen 1/8-Kreissektor (im Dreieck mit y <= x) erfolgen würden?

asc

Einzig noch zwei Minianmerkungen, das ist aber eher eine Geschmackssache, nach meiner Erfahrung erhöht dies in großen Projekten aber die Lesbarkeit etwas.

1. ich habe public:/private nicht auf der gleichen Ebene wie die Methoden/Attribute (Von der Einrückung her ist dies schneller zu erkennen, was zu welchen Teil gehört).

2. hinter if/else schreibe ich aus dem gleichen Grund niemals direkt die Anweisung in die gleiche Zeile, selbst bei 1-Zeilen Anweisungsblöcken (Statt dessen eingerückt in Nächster). Auch wenn ich ursprünglich hier keinen Unterschied beim Lesen erwartet hätte, ist mir beim schnellen Fehlersuchen ohne die Einrückung etwas einmal entgangen.

Zudem würde ich dir davon abraten Tabulatoren in dem Forum zu verwenden (in den meisten Projekten, in denen ich gearbeitet habe wurde auch der Tabulator immer durch eine fixe Leerzeilenanzahl ersetzt).

alaundo schrieb:

pow(y,2) solltest du schleunigst durch y*y ersetzen. Das dürfte deutlich schneller sein.

Tatsächlich, aber warum?

Ich weiß nicht ob die Aussage stimmt, könnte mir es aber gut vorstellen. Bei ^2 kann man auf eine Multiplikation ausweichen (Spezialfall), pow ist aber für den allgemeinen Fall mit beliebigen Exponenten ausgelegt, und die Berechnung könnte aufwendiger sein.

camper schrieb:

alaundo schrieb:
double pi = 4.0*treffer/wuerfe;		//mal 4 wegen Wurf auf Viertelkreis
Mit diesem Kommentar stimmt etwas nicht. Wäre der Faktor wirklich ein anderer, wenn die Würfe auf den Vollkreis oder z.B. nur auf einen 1/8-Kreissektor (im Dreieck mit y <= x) erfolgen würden?

Eigentlich wird ja nicht auf den Viertelkreis sondern auf das Einheitsquadrat geworfen. Und das überdeckt halt nur einen Viertelkreis, sodass im Endeffekt pi/4 angenähert wird, also stimmt das schon, glaube ich.

oenone

pow ist für ganzzahlige Exponenten nicht effizient. Deshalb sollte man es nur für Exponenten vom Typ float/double benutzen.

Statt den Zufallswert durch Multiplikation und Division auf 3 Nachkommastellen zu runden, könntest du gleich Integer-Werte von 0-1000 nehmen. Dadurch sparst du dir die Fließkommaberechnungen und Ungenauigkeiten. Dementsprechend wird auch die Berechnung ob innerhalb des Einheitskreises effizienter.

Skym0sh0

oenone schrieb:

pow ist für ganzzahlige Exponenten nicht effizient. Deshalb sollte man es nur für Exponenten vom Typ float/double benutzen.

Und was ist bei x^84287423784278342378 ?
Manuell ausschreiben?
Schnell nen for-Loop schreiben (vllt mit Funktion)?
std::pow benutzen?

Jockelx

huppel schrieb:

camper schrieb:
alaundo schrieb:
double pi = 4.0*treffer/wuerfe;		//mal 4 wegen Wurf auf Viertelkreis
Mit diesem Kommentar stimmt etwas nicht. Wäre der Faktor wirklich ein anderer, wenn die Würfe auf den Vollkreis oder z.B. nur auf einen 1/8-Kreissektor (im Dreieck mit y <= x) erfolgen würden?
Eigentlich wird ja nicht auf den Viertelkreis sondern auf das Einheitsquadrat geworfen. Und das überdeckt halt nur einen Viertelkreis, sodass im Endeffekt pi/4 angenähert wird, also stimmt das schon, glaube ich.

Es wurde ja nicht der Code beanstandet, sondern der Kommentar.
Würden die Darts auf [-1,1]x[-1,1] geworfen, würde trotzdem pi/4 angenähert.
Hat mit dem Viertelkreis im Kommentar nix zu tun.

alaundo

Hallo zusammen!

Mit diesem Kommentar stimmt etwas nicht. Wäre der Faktor wirklich ein anderer, wenn die Würfe auf den Vollkreis oder z.B. nur auf einen 1/8-Kreissektor (im Dreieck mit y <= x) erfolgen würden?

Ja, beispielsweise 1/8-Kreissekter im Einheitsquadrat hat genau den halben Flächeninhalt, dementsprechend würde pi/8 angenähert (liefert mein Programm auch ). Dass es vom Winkel abhängt sieht man schon daran, dass bei sehr sehr kleinen Winkeln die Trefferchance auch nur noch sehr klein ist, also ein größerer Faktor multipliziert werden muss.

Klar: wenn ich den Vollkreis im (vergrößerten) Quadrat nehme, bleibt der Faktor 1/4.

Danke für die Tipps mit pow, ich werde wohl einfach die Multiplikationen, die man noch schnell aufschreiben kann, mit * machen, und bei Fließkommazahlen oder großen Potenzen mit pow. Wie würde das bei Zweierpotenzen mit << gehen?

Die anderen Anmerkungen (Einrückungen, if-Schleife bei 0, Einrückung und Tabulatoren) baue ich ein.

Danke!
alaundo

asc

alaundo schrieb:

if-Schleife

if != Schleife

alaundo

Äh ja, klar :p

Skym0sh0

Die << und >> sind Bitshiftoperatoren:

for(unsigned int i = 0; i < sizeof(unsigned int)*8; ++i)
    std::cout << (1 << i) << " vs. " << std::pow(2, i) << std::endl;

Es werden die Bits eines Datums einfach nach links* geschoben. Praktische ist das eine Multiplikation* mit der i-ten Zweierpotenz. Dabei gibt es dann noch so Späße, was mit Vorzeichen passiert oder bei Überläufen.

Meiner Erfahrung nach braucht man das relativ selten. Und wenn mans braucht, dann weiss mane rstens schon ziemlich genau was man macht. In den meisten anderen Fällen kann der Compiler das selsbt einbauen (z.B. bei einer simplem Multiplikation/Division mit 2).

*: rechts bzw. Division

volkard

Skym0sh0 schrieb:

Die << und >> sind Bitshiftoperatoren:

for(unsigned int i = 0; i < sizeof(unsigned int)*8; ++i)
    std::cout << (1 << i) << " vs. " << std::pow(2, i) << std::endl;

Hier war '\n' statt endl gemeint.