Erster C++-Gehversuch, Feedback erwünscht

Columbo

asdfasd schrieb:

empty |= !cell;
sieht komisch aus. Warum nicht einfach so?
empty = !cell;

Weil das etwas anderes tut. Was wenn in empty true gespeichert ist und !cell den Wert false hat?

Besser wäre jedoch gleich den logischen Operator zu verwenden:

empty ||= !cell;

Anstelle der rohen Funktionszeiger könntest du mir mal std::function anschauen

Ne, nicht gut. function ist hier unpassend.

Fytch

Arcoth schrieb:

Besser wäre jedoch gleich den logischen Operator zu verwenden:
empty ||= !cell;

Diesen Operator gibt es nicht.

volkard

#pragma once

#include <cassert>
#include <array>

template<int Size>
class MagicSquare {
//Hier sollten die Attribute stehen, damit man 
public:

	inline void unset_cell(int row, int col) {
		unset_cell(cell_id(row, col));//hier weiß, um was es geht. 
		//So musste ich erst unten nachschauen. Ah, Du baust so ein Array2D. 
		//Naja, eine simple int& at(int x,int y) hötte das auch abgedeckt. 
	}

	inline void set_cell(int row, int col, int val) {//übrigens brauchste inline nie hinzuschreiben, 
	//das macht der compiler eh wie er will. 
		set_cell(cell_id(row, col), val);
	}

	inline int get_cell(int row, int col) {
		return get_cell(cell_id(row, col));
	}

	inline void replace_cell(int row, int col, int val) {
		replace_cell(cell_id(row, col), val);
	}
	//Die letzten VIER Funktionen waren daher ein wenig übertrieben. 

	int get_progress() {
		int prog = 0;
		for (int idx = 0; idx < FIELDS; ++idx) {//Das ist lahm. Lieber prog als Attribut mitführen. 
			if (field[idx]) {
				++prog;
			}
		}
		return prog;
	}

	void resolve(void (*consumer)(MagicSquare<Size> &)) {//funktionszeiger statt funktionsobjekt ist lahmer. 
		resolve(consumer, get_progress());//und was das hier soll, weiß ich gar nicht. 
	}

	void resolve(void (*consumer)(MagicSquare<Size> &), int progress) {//Das nervt, daß Du gleichname Wrapper 
	//schreibst, satt Dir als erste Zeile eine Variable zu besorgen. Immer noch nicht klar, was "resolve" heißt. 
		if (progress == FIELDS) {
			consumer(*this);
		}

		int firstUnusedCell = 0;
		while (field[firstUnusedCell]) {
			++firstUnusedCell;//Bis fast sicher, das würde vom Attribut prog vollkommen übernommen werden können. 
			//Momentchen, warum nicht sowas ähnliches wie fisher-yates-shuffle? 
			//Oder std::next_permutation. 
		}

		for (int idx = 1; idx < FIELDS + 1; ++idx) {
			if (usedNumbers[idx]) {
				continue;
			}
			replace_cell(firstUnusedCell, idx);

			if (check_square()) {
				resolve(consumer, progress + 1);//Ah, der rekursive Aufruf. Ohne den Wrapper?
			}
		}
		unset_cell(firstUnusedCell);
	}

	bool check_square() {
		for (int idx = 0; idx < Size; ++idx) {
			if (!check_row(idx) || !check_col(idx)) {
				return false;
			}
		}

		if (!check_diag1() || !check_diag2()) {
			return false;
		}
		return true;
		//Die war hübsch einfach. 
	}

	bool check_diag1() {
		int sum = 0;
		bool empty = false;
		for (int idx = 0; idx < Size; ++idx) {
			int cell = get_cell(idx, idx);
			sum += cell;
			empty |= !cell;//Die Zeile wird Arcoth gefallen. 
			if (sum > ROWSUM) {
				return false;
			}
		}

		return empty || sum == ROWSUM;
	}

	bool check_diag2() {
		int sum = 0;//Ganzschön gleich die letzten 2 Funktionen. Kann man zusammenlegen. 
		bool empty = false;
		for (int idx = 0; idx < Size; ++idx) {
			int cell = get_cell(idx, Size - 1 - idx);
			sum += cell;
			empty |= !cell;
			if (sum > ROWSUM) {
				return false;
			}
		}

		return empty || sum == ROWSUM;
	}

	bool check_col(int col) {
		int sum = 0;//Ganzschön gleich die letzten 3 Funktionen. Kann man zusammenlegen. 
		bool empty = false;
		for (int row = 0; row < Size; ++row) {
			int cell = get_cell(row, col);
			sum += cell;
			empty |= !cell;
			if (sum > ROWSUM) {
				return false;
			}
		}

		return empty || sum == ROWSUM;
	}

	bool check_row(int row) {
		int sum = 0;//Ganzschön gleich die letzten 4 Funktionen. Kann man zusammenlegen. 
		bool empty = false;
		for (int col = 0; col < Size; ++col) {
			int cell = get_cell(row, col);
			sum += cell;
			empty |= !cell;
			if (sum > ROWSUM) {
				return false;
			}
		}

		return empty || sum == ROWSUM;
	}

private:
	enum {//Nee, static const
		FIELDS = Size * Size,
		ROWSUM = Size * (Size * Size + 1) / 2//Gut, die vorzuberechnen
	};

	inline int cell_id(int row, int col) {
		assert(row < Size);
		assert(col < Size);
		return row * Size + col;
	}

	inline void set_cell(int cell, int value) {
		assert(field[cell] == 0);//ahm dafür in set, unset, replace aufgeteilt. 
		assert(usedNumbers[value] == false);
		field[cell] = value;
		usedNumbers[value] = true;
	}

	inline void unset_cell(int cell) {
		usedNumbers[field[cell]] = false;
		field[cell] = 0;
	}

	inline int get_cell(int cell) {
		return field[cell];
	}

	inline void replace_cell(int cell, int value) {
		usedNumbers[field[cell]] = false;
		usedNumbers[value] = true;
		field[cell] = value;
	}

	std::array<int, FIELDS> field = { };
	std::array<bool, FIELDS + 1> usedNumbers = { };
};

#include <iostream>
#include <array>
#include <cstdio>

#include "MagicSquare.hpp"

template<int Size>
void dumpSquare(MagicSquare<Size> &square) {
	printf("----------------\n");
	for (int row = 0; row < Size; row++) {//viele bevorzugen != und präinkrement
		for (int col = 0; col < Size; col++) {
			printf("%2d ", square.get_cell(row, col));//printf
		}
		printf("\n");
	}
}

int main() {
	MagicSquare<4> square;
	square.resolve(&dumpSquare);
	return 0;
}

Stummi

Arcoth schrieb:

[*]Du verletzt DRY.

Das war mir klar, aber nicht wie ich es besser machen könnte. Danke für den Verweis auf Lambdas und Funktionszeiter, da werde ich mich nochmal genauer einlesen. Dein Codebeispiel bringt mir aber einen Fehler:

../MagicSquare.hpp:93:43: error: ‘this’ was not captured for this lambda function

Grüße,
Stummi

TNA

asdfasd schrieb:

Und genaugenommen ist #pragma once kein Standardkonformes C++ (oder hat sich das inzwischen geändert?)

Pragma-Direktiven sind definitionsgemäß implementationsspezifisch und damit kein Teil des C++ Standards auch wenn diese mittlerweile meines Wisens nach von allen gängigen Compilern unterstützt wird.

Nathan

Stummi schrieb:

../MagicSquare.hpp:93:43: error: ‘this’ was not captured for this lambda function

Ja, du musst noch this capturen, sonst kann das Lambda keine Memberfunktionen aufrufen.
Einfach this in die eckigen Klammern schreiben.

Columbo

asfdlol schrieb:

Diesen Operator gibt es nicht.

Tatsächlich! Ich werd' verrückt! Bist du gerade in der Zeit zurückgereist und hast diesen Operator aus jeglichen Standardisierungen von C und C++ entfernt?

Gut, schreiben wir es aus, immer noch besser:

empty = empty || !cell;

Die Zeile wird Arcoth gefallen.

Ist es vom Standard definiert dass true | false wiederum true oder false ergibt? Siehe auch hier.

Dein Codebeispiel bringt mir aber einen Fehler:

Upsi. Schreib das Lambda in beiden Fällen zu

[this] (int idx) {return get_cell(idx, idx);}

um, und probier' noch mal.

Columbo

funktionszeiger statt funktionsobjekt ist lahmer.

Ein Funktionszeiger ist ein Funktionsobjekt. Ein Funktionsobjekt ist dadurch definiert dass man es wie eine Funktion aufrufen kann. Und was heißt "Lahm"*? Ich hoffe "uncooler".

Übrigens:

if (!check_diag1() || !check_diag2()) {
            return false;
        }
        return true;

return check_diag1() && check_diag2();

De Morgansche Gesetze und so.

* Außenverteidiger!

Fytch

Arcoth schrieb:

Tatsächlich! Ich werd' verrückt! Bist du gerade in der Zeit zurückgereist und hast diesen Operator aus jeglichen Standardisierungen von C und C++ entfernt?

Das hab ich damals schon vor Ort vollbracht.
Spass beiseite. Die Tatsache, dass diese Operatoren und dass der ^^-Operator (Logisch-XOR) fehlen haben mich schon aufgeregt.

Arcoth schrieb:

Ist es vom Standard definiert dass true | false wiederum true oder false ergibt? Siehe auch hier.

Darüber habe ich mir auch schon Gedanken gemacht. Einmal hat jemand hier im Forum einen Ausschnitt aus dem Standard gepostet (das war SeppJ glaub ich, ich finde den Beitrag jedoch nicht mehr, das ist ein Weilchen her, vermutlich noch auf einem meiner alten Accouts), dass dies (und dasselbe mit XOR) wohldefiniert ist. Nachdem camper in dem von dir verlinkten Thread etwas anderes (und genaueres) geschrieben hat bin ich unsicher.

volkard

Arcoth schrieb:

funktionszeiger statt funktionsobjekt ist lahmer.

Ein Funktionszeiger ist ein Funktionsobjekt. Ein Funktionsobjekt ist dadurch definiert dass man es wie eine Funktion aufrufen kann. Und was heißt "Lahm"*? Ich hoffe "uncooler".

Übrigens:
if (!check_diag1() || !check_diag2()) {
            return false;
        }
        return true;
return check_diag1() && check_diag2();
De Morgansche Gesetze und so.

* Außenverteidiger!

Nöö, das war schon sehr gut so.

volkard

asfdlol schrieb:

Arcoth schrieb:

Tatsächlich! Ich werd' verrückt! Bist du gerade in der Zeit zurückgereist und hast diesen Operator aus jeglichen Standardisierungen von C und C++ entfernt?

Das hab ich damals schon vor Ort vollbracht.
Spass beiseite. Die Tatsache, dass diese Operatoren und dass der ^^-Operator (Logisch-XOR) fehlen haben mich schon aufgeregt.

Den ^^ gibts, nur heißt er !=.

Fytch

volkard schrieb:

Den ^^ gibts, nur heißt er !=.

Ich hab zwar keine Ahnung wie bool s miteinander verglichen werden, aber wenn die oberen Bits über dem achten unspezifiziert sind, dann kann so ein != doch schief gehen, oder? Oder wird beim Vergleich nur das LSB betrachtet?

Edit: Ich sage besser nichts dazu, ich weiss nicht was der Standard über bool s aussagt. Nicht auf mich hören also!

Nathan

asfdlol schrieb:

volkard schrieb:

Den ^^ gibts, nur heißt er !=.

Ich hab zwar keine Ahnung wie bool s miteinander verglichen werden, aber wenn die oberen Bits über dem achten unspezifiziert sind, dann kann so ein != doch schief gehen, oder? Oder wird beim Vergleich nur das LSB betrachtet?

Edit: Ich sage besser nichts dazu, ich weiss nicht was der Standard über bool s aussagt. Nicht auf mich hören also!

Ich geh einfach mal davon aus, dass Vergleiche für bools schon definiert sind und zwei gleiche immer gleich sind und nicht auf einmal verschieden, wenn da anderer Müll ist. Das wäre sonst ziemlich blöd.

camper

asfdlol schrieb:

Arcoth schrieb:

Ist es vom Standard definiert dass true | false wiederum true oder false ergibt? Siehe auch hier.

Darüber habe ich mir auch schon Gedanken gemacht. Einmal hat jemand hier im Forum einen Ausschnitt aus dem Standard gepostet (das war SeppJ glaub ich, ich finde den Beitrag jedoch nicht mehr, das ist ein Weilchen her, vermutlich noch auf einem meiner alten Accouts), dass dies (und dasselbe mit XOR) wohldefiniert ist. Nachdem camper in dem von dir verlinkten Thread etwas anderes (und genaueres) geschrieben hat bin ich unsicher.

true | false ergibt 1
Was ich in dem Thread geschrieben habe bezog sich auf die ABI - mithin das was nach der Übersetzung in Assembler-/Maschinencode zu beachten ist. Mit dem Verhalten auf der Ebene von C++ hat das nichts zu tun: | führt erst mal die gewöhnlich arithmetischen Konvertierungen auf beiden Operanden aus (weil es nur für int und größere integrale Typen definiert ist). Und die Konvertierung von bool nach int ergibt immer 1 bzw. 0 für true bzw. false.