Fehlermeldung bei Zugriff auf dynamisches Array

Ich generiere meine Zufallszahlen immer mit randutils.hpp von M.E. O'Neill: http://www.pcg-random.org/posts/ease-of-use-without-loss-of-power.html

wob schrieb:

Ein Kommentar noch zu outs Code. Ich würde in fill_field die beiden Variablen
random_device rd;
    mt19937 mt( rd() );
static machen. Den rng sollte man ja nur einmalig initialisieren und nicht jedes mal neu. Gut, da du die Funktion nur 1x aufrufst, ist das egal.

Stimmt

Thorsten54

Erstmal vielen dank an euch das ihr mir bei meinem problem helft
Das mit den Vektoren werde ich mir gleich mal anschauen das ist eh mehr so ein projekt um den umgang mit arays zu üben deshalb möchte ich hier vorerst keine Vektoren einbauen ich hab den code jetzt auch zum laufen gebracht aber leider ist die berechnung der nächsten generation falsch vieleicht kann mir ja jemand helfen

int main(int argc, char* argv[]) {

	int x=0, y=0,i,j,temp2=0,temp3=0;
	char temp;
	srand((unsigned)time(0));

	while ((y <= 0)|(x <= 0)) {
		cout << "Wie gro\xE1 soll das Feld werden?" << endl << "X<<";
		cin >> x;
		cout << "Y<<";
		cin >> y;
		system("cls");
		if ((x <= 0)|(y <= 0))
			cout << "Die Werte m\x81 \bssen \x81 \bber Null liegen!" << endl;
	}

	  int **Feld = new(int*[x]) ;
	for (i = 0; i < x; i++)
		Feld[i] = new(int[y]);

	for (i = 0; i < x; i++) {
		for (j = 0; j < y; j++) {
			temp= rand() % 100;
			if (temp % 7 == 1)
				Feld[i][j] = 1;
			else
				Feld[i][j] = 0;
		}
	}

	while (temp2 == 0) {

		cout << "Starten:enter" << endl << "Beenden:ESC" << endl << "<<:";
		temp = _getch();

		switch (temp) {

		case 13://Start
			temp2 = 1;
			system("cls");
			break;

		case 27://Beenden
			return 0;
			break;

		default:
			system("cls");
			temp2 = 0;
			break;
		}
	}

	temp =  0;
	temp2 = 0;
	temp3 = 0;
	while (true) {

		if (_kbhit()) {

			temp=_getch();

			if (temp == 27) 
				return 0;

			system("cls");

			cout << temp2++ << ". Generation" << endl << endl;
			for (i = 0; i < x; i++) {
				for (j = 0; j < y; j++) {
					if (Feld[i][j] == 0) {
						cout << " ";
					}
					else {
						cout << "#";
					}
				}
				cout << endl;
			}

			for (i = 0; i < x; i++) {

				for (j = 0; j < y; j++) {

					if ((i > 0) && (j > 0) &&				(Feld[i - 1][j - 1] == 1)) temp3++;

					if ((i > 0) &&							(Feld[i - 1][j] == 1)) temp3++;

					if ((i > 0) && (j < (y - 1)) &&			(Feld[i - 1][j + 1] == 1)) temp3++;

					if ((j > 0) &&							(Feld[i][j - 1] == 1)) temp3++;

					if ((j < (y - 1)) &&					(Feld[i][j + 1] == 1)) temp3++;

					if ((i < (x - 1)) && (j > 0) &&			(Feld[i + 1][j - 1] == 1)) temp3++;

					if ((i < (x - 1)) &&					(Feld[i + 1][j] == 1)) temp3++;

					if ((i < (x - 1)) && (j < (y - 1)) &&   (Feld[i + 1][j + 1] == 1)) temp3++;

					if ((temp3 < 4) && (temp3 > 1)) {
						if ((Feld[i][j] == 0) && (temp3 == 3))
							Feld[i][j] = 1;

						if(!(Feld[i][j]==0))
							Feld[i][j] = 1;
					} else {
						Feld[i][j] = 0;
					}
				}
			}
		}
	}
	return 0;
}

wob

Ich schrieb:

Und dann wird der neue Zustand für alle Felder gleichzeitig ermittelt. Das heißt für dich, dass du eine Kopie* des Feldes erstellen musst, wo du die neuen Zustände speicherst, denn ansonsten wird ja der geänderte Zustand eines Punktes schon für die weitere Berechnung der neuen Felder genutzt.

Das gilt immer noch.

out schrieb außerdem, dass du Warnungen beachten sollst! Insbesondere ist da immer noch die Warnung zu deinen news:

// WARNING: When type is in parentheses, array cannot have dynamic size

Das gilt ebenfalls immer noch! Nimm die Klammern beim new weg.

Außerdem: logisches Oder ist || , logisches Und ist && (nicht | bzw. & ).

Und ein weiterer, davon unabhängiger Rat: deklariere die Variablen immer erst dort, wo du sie brauchst. Insbesondere deklariere die Schleifenvariablen i und j erst in den Schleifen, nicht oben für das gesamte Programm.

Wichtigster Rat: das ist zwar C++, aber kein idiomatisches C++! Ich würde einfach einen vector<vector<int>> verwenden oder sogar einen einfachen vector<int> und die Indizes von Hand berechnen. Gut, wenn du wirklich new und herumgepointere lernen willst, kannst du das natürlich erstmal so lernen. Du hast mit diesem Programm zum Beispiel schon ein Speicherleck (zu jedem new gehört auch ein delete). Ich frage mich nur: warum immer erst einen komplizierten, fehleranfälligen Weg lernen, wenn es auch einfach und sicher geht?

hustbaer

Drei Sterne...
Bei Hotels is das einer zu wenig.
Beim C++ Programmieren sind es zwei zu viel.

muemmel

hustbaer schrieb:

Drei Sterne...
Bei Hotels is das einer zu wenig.
Beim C++ Programmieren sind es zwei zu viel.

Thorsten54

Ich hab mal versucht alle deine verbesserungen umzusetzen allerdings funktioniert es immer noch nicht richtig(das mit den Zeigern ändere ich später auch noch)

#include<iostream>
#include<time.h>
#include<conio.h>
using namespace std;

int main(int argc, char* argv[]) {

	int x = 0, y = 0, i, j;

	srand((unsigned)time(0));

	while ((y <= 0)||(x <= 0)) {
		cout << "Wie gro\xE1 soll das Feld werden?" << endl << "X<<";
		cin >> x;
		cout << "Y<<";
		cin >> y;
		system("cls");
		if ((x <= 0)||(y <= 0))
			cout << "Die Werte m\x81 \bssen \x81 \bber Null liegen!" << endl;
	}

	  bool **Feld = new(bool*[x]) ;
	for (i = 0; i < x; i++)
		Feld[i] = new(bool[y]);

	bool **tempf = new(bool*[x]);
	for (i = 0; i < x; i++)
		tempf[i] = new(bool[y]);

	char temp;

	for (i = 0; i < x; i++) {
		for (j = 0; j < y; j++) {
			temp= rand() % 100;
			if (temp % 7 == 1)
				Feld[i][j] = 1;
			else
				Feld[i][j] = 0;
		}
	}

	for (i = 0; i<x; i++)
		for (j = 0; j < y; j++) {
			tempf[i][j] = Feld[i][j];
		}

	bool temp2 = 0;

	while (temp2 == 0) {

		cout << "Starten:enter" << endl << "Beenden:ESC" << endl << "<<:";
		temp = _getch();

		switch (temp) {

		case 13://Start
			temp2 = 1;
			system("cls");
			break;

		case 27://Beenden
			return 0;
			break;

		default:
			system("cls");
			temp2 = 0;
			break;
		}
	}

	temp =  0;
	temp2 = 0;
	short temp3 = 0;
	while (true) {

		if (_kbhit()) {

			temp=_getch();

			if (temp == 27) 
				return 0;

			system("cls");

			cout << temp2++ << ". Generation" << endl << endl;
			for (i = 0; i < x; i++) {
				for (j = 0; j < y; j++) {
					if (tempf[i][j] == 0) {
						cout << " ";
					}
					else {
						cout << "#";
					}
				}
				cout << endl;
			}

			for(i=0;i<x;i++)
				for (j = 0; j < y; j++) {
					Feld[i][j] = tempf[i][j];
				}

			for (i = 0; i < x; i++) {

				for (j = 0; j < y; j++) {

					if ((i > 0) && (j > 0) &&				(Feld[i - 1][j - 1] == 1)) temp3++;

					if ((i > 0) &&							(Feld[i - 1][j] == 1)) temp3++;

					if ((i > 0) && (j < (y - 1)) &&			(Feld[i - 1][j + 1] == 1)) temp3++;

					if ((j > 0) &&							(Feld[i][j - 1] == 1)) temp3++;

					if ((j < (y - 1)) &&					(Feld[i][j + 1] == 1)) temp3++;

					if ((i < (x - 1)) && (j > 0) &&			(Feld[i + 1][j - 1] == 1)) temp3++;

					if ((i < (x - 1)) &&					(Feld[i + 1][j] == 1)) temp3++;

					if ((i < (x - 1)) && (j < (y - 1)) &&   (Feld[i + 1][j + 1] == 1)) temp3++;

					if ((temp3 < 4) && (temp3 > 1)) {
						if ((Feld[i][j] == 0) && (temp3 == 3))
							tempf[i][j] = 1;

						if(!(Feld[i][j]==0))
							tempf[i][j] = 1;
					} else {
						tempf[i][j] = 0;
					}
				}
			}
		}
	}
	delete[] tempf;
	delete[] Feld;
	return 0;
}

wob

Ich hab mal versucht alle deine verbesserungen umzusetzen allerdings funktioniert es immer noch nicht richtig

Soll ich lachen oder weinen?

Wenn aus

int **Feld = new(int*[x]) ;

durch Weglassen der Klammern

bool **Feld = new(bool*[x]) ;

wird, dann frage ich mich schon, inwiefern das "umgesetzt" ist.

Auch sagte ich dir, dass du den Gültigkeitsbereich deiner Variablen begrenzen solltest. Das gilt nicht nur für i und j, sondern auch für temp3! Bei temp3 ist es sogar ein grober Fehler, die so weit außen zu deklarieren. Du musst sie ja für jedes Feld, das du neu berechnest, auf 0 setzen. Also schieb das "short temp3=0" direkt vor das if ((i > 0) && (j > 0) ...

Variablennamen wie "temp3" sind besch...! Vergib gute Namen!

Und dann: vor dem Neuberechnen brauchst du mit tempf nichts tun, du musst lediglich sicherstellen, dass jedes Element von tempf auch neu gesetzt wird. In deinem

if ((temp3 < 4) && (temp3 > 1)) {
                        if ((Feld[i][j] == 0) && (temp3 == 3))
                            tempf[i][j] = 1;

                        if(!(Feld[i][j]==0))
                            tempf[i][j] = 1;
                    } else {
                        tempf[i][j] = 0;
                    }

ist das nicht immer der Fall. Einfacher: setzt tempf[i][j]=0 und mach dann nur die ifs, die es auf 1 setzen. Danach musst du dann in einer Schleife tempf in Feld kopieren.

Und dann sollte es funktionieren. (Kann es mangels Windows nicht testen - denn system, _khbit und _getch sind Windows-spezifisch)

PS: hast du eigentlich schon gelernt, was Funktionen sind? Wenn nein, dann unbedingt nachholen. Wenn ja, dann unbedingt benutzen, ggf. nochmal wiederholen! Und bezüglich der deletes: du deletest nur die "äußeren" Arrays, aber nicht die inneren. Du hast die inneren manuell mit einer Schleife erzeugt, also musst du sie auch so wieder löschen. Ja, das ist aufwendig. Aber du wolltest es ja nicht einfach haben und std::vector nutzen

Thorsten54

das mit den funktionen habe ich ja versucht leider funktioniert das nicht so ganz weshalb ich es kurzer hand weggelassen habe und ich habe mich inzwichen auch mal in diese vektoren eingelesen dabei bin ich auf diese neuen arrays gestoßen

array<datentyp,anzahl> name

gibt es da einen entscheidenten unterschied zu den Vektoren außer das die vektoren automatisch vergrößert werden,und warum sieht man diese neuen arrays nicht so oft in Foren

Th69

Bei array<datentyp, anzahl> muß die anzahl schon zur Kompilierungszeit feststehen (da es ein Template-Parameter ist) - in deinem Programm fragst du aber explizit zur Laufzeit nach der Größe (daher vector<>).

Thorsten54 schrieb:

warum sieht man diese neuen arrays nicht so oft in Foren

Weil häufig Fragen in Foren von ***-Programmierern gestellt werden und nicht von solchen, die vernünftige Datenstrukturen kennen.

Thorsten54

ich habe das jetzt mal mit arays gemacht die ich eine classe abgekapselt habe hier ist die member die die nächste generation berechnet hier müsste der fehler liegen

void berechnen() {

		for (i = 0; i < x; i++) {
			for (j = 0; j < y; j++) {

				zellen = 0;
				if ((i > 0) && (j > 0) && (Feld[i - 1][j - 1] == 1))zellen++;

				if ((i > 0)&&(Feld[i - 1][j] == 1))zellen++;

				if ((i > 0) && (j < (y - 1)) && (Feld[i - 1][j + 1] == 1))zellen++;

				if ((j > 0) && (Feld[i][j - 1] == 1))zellen++;

				if ((j < (y - 1)) && (Feld[i][j + 1] == 1))zellen++;

				if ((i < (x - 1)) && (j > 0) && (Feld[i + 1][j - 1] == 1))zellen++;

				if ((i < (x - 1)) && (Feld[i + 1][j] == 1))zellen++;

				if ((i < (x - 1)) && (j < (y - 1)) && (Feld[i + 1][j + 1] == 1))zellen++;

				if ((zellen < 4) && (zellen > 1) && (Feld[i][j] == 1)) {
					Temp[i][j] = 1;
				}

				if (zellen == 3) {
					Temp[i][j] = 1;
				}

			}

			return;
		}

DJohn

Hallo

bist Du sicher, dass Du uns den Code zeigst, der den Fehler produziert? Welcher Fehler tritt den noch auf?

berechne ist momentan keine Member, sondern eine freie Funktion?
i, j, und zellen sollten lokale Variblen sein, sind aber globale Variablen, warum?
x, y, Feld und Temp sind ebenfalls globale Varibale, aber Du schreibst Du hast das in eine Klasse gekapselt?

Die Logik sieht sonst soweit ok aus, wenn man davon ausgeht, dass Feld und Temp jeweils vom richtigen Typ sind und die Größen x,y haben. Außerdem müssen alle Felder von Temp mit 0 vorbelegt sein.

Thorsten54

nein nein berechnen und alle variablen(außer x,y das sind globale const) sind member der classe field ich habe aber
ich habe jetzt nur den teil hochgeladen in dem ich den Fehler vermute
hier kommt mal alles:

#include<iostream>
#include<time.h>
#include<conio.h>
using namespace std;
int const  y = 2, x = 2;

class field {
public:
	bool Feld[x][y];
	bool Temp[x][y];
	short i, j,zellen,sp;
	int generation;

	void ausgeben() {
		system("cls");
			cout << generation++ << ".Generation" << endl;
		for (i = 0; i < x; i++) {
			for (j = 0; j < y; j++) {
				if (Feld[i][j] == 0) {
					cout << " ";
				}
				else {
					cout << "#";
				}
			}
			cout << endl;
		}
		return;
	}
	void berechnen() {

		for (i = 0; i < x; i++) {
			for (j = 0; j < y; j++) {

				zellen = 0;
				if ((i > 0) && (j > 0) && (Feld[i - 1][j - 1] == 1))zellen++;

				if ((i > 0)&&(Feld[i - 1][j] == 1))zellen++;

				if ((i > 0) && (j < (y - 1)) && (Feld[i - 1][j + 1] == 1))zellen++;

				if ((j > 0) && (Feld[i][j - 1] == 1))zellen++;

				if ((j < (y - 1)) && (Feld[i][j + 1] == 1))zellen++;

				if ((i < (x - 1)) && (j > 0) && (Feld[i + 1][j - 1] == 1))zellen++;

				if ((i < (x - 1)) && (Feld[i + 1][j] == 1))zellen++;

				if ((i < (x - 1)) && (j < (y - 1)) && (Feld[i + 1][j + 1] == 1))zellen++;

				if ((zellen < 4) && (zellen > 1) && (Feld[i][j] == 1)) {
					Temp[i][j] = 1;
				}

				if (zellen == 3) {
					Temp[i][j] = 1;
				}

			}

			return;
		}
	}
	void füllen() {
		srand((unsigned)time(0));
		for (i = 0; i < x; i++) {
			for (j = 0; j < y; j++) {
				Feld[i][j] = 0;
				sp = rand() % 100;
				//if (sp % 4 == 2) {
					Feld[i][j] = 1;

				//}
			}
		}
	return;
	}
	void gleichen() {
		for (i = 0; i < x; i++) {
			for (j = 0; j < y; j++) {
				Feld[i][j] = Temp[i][j];
			}
		}
		return;
	}
	void fullen() {
		for (i = 0; i < x; i++) {
			for (j = 0; j < y; j++) {
				Temp[i][j] = 0;
			}
		}
	return;
	}

};

int main(int argc, char**argv[]) {

	field f;
	bool kontrolle = 1;
	short eingabe;

	f.füllen();//Füllt Feld mit zufälligen true oder false

	while (kontrolle) {

		cout << "Start:Enter" << endl << "Verlassen:ESC" << endl << "Optionen:O" << endl << "<<:";

		eingabe = _getch();

				switch (eingabe) {

		case 13://Weiter

			kontrolle = 0;

			break;

		case 'O':
		case 'o':

			break;

		case 27://Beenden
			return 0;
			break;

		default:

			system("cls");

			break;
		}
	}

	f.generation = 0;

	while (true) {
		if (_kbhit()) {

			eingabe = _getch();

			if (eingabe == 27)
				return 0;

			f.ausgeben();//Gibt Feld aus
			f.fullen();//Füllt Temp mit nullen aus
			f.berechnen();//Berechnet nächste genation in Temp
			f.gleichen();//Kopiert Temp in Feld
		}
	}
	return 0;
}

DJohn

Hallo,

Du könntest schon noch dazu sagen, welchen Fehler Du in Deinem aktuellen Code beobachtest. Dann ist das Suchen deutlich einfacher. Du kannst auch Deinen Code debuggen und Schritt für durchgehen. Dann siehst Du auch gleich, an welcher Stelle etwas passiert, was Du so nicht erwartet hast.
Das return in Zeile 63, ist an der falschen Stelle. Das gehört in die Zeile unter die nächste schliessende geschweifte Klammer, sonst brichst Du die for(i=...) -Schleife vorzeitig ab. Das return ist sowieso unnötig, Du kannst es gleich ganz weglassen.

Nach der 0.Generation gibt es keine lebenden zellen mehr
Thorsten54

wob

Du hast in deinem "berechnen" ein return versteckt, das da nicht hingehört. Kompiliere mit Warnungen!

Ich habe den Code nur mal schnell dem Compiler zum Angucken gegeben und schon sehe ich bei deiner Loop for (i = 0; i < x; i++) { die Warnung "Loop will run at most once (loop increment never executed) [-Wunreachable-code-loop-increment]"

Außerdem sollen deine Loop-Variablen LOKALE Variablen sein. Wie weit deine Loop gerade fortgeschritten ist, ist NICHT ein beschreibendes Element deiner Klasse!

Ja vielen dank aber ich weiß nicht wie ich die warnungen in visual c++ 2017 so konfigurie das er mich auf sowas Hinweist ich habe das schon gegoogelt aber da ließ sich nichts finden und die codeanalyse
hab ich auch so eingestellt das alle möglichen fehler angezeigt werden
Vielleicht könnte mir jemand an der aktuellen version beschreiben wie man das einstellt
Thorsten54

wob

Schau dir mal https://www.youtube.com/watch?v=zMrP8heIz3g&index=6&list=PLs3KjaCtOwSbij6EOk7K-ZgKKcxH7yVHC (und die Nachfolgende Episode) an. Da wird erklärt, wie man Visual Studio 2017 so einrichtet, dass man gutes Tooling hat.