• Hallo liebe Userinnen und User,

    nach bereits längeren Planungen und Vorbereitungen sind wir nun von vBulletin auf Xenforo umgestiegen. Die Umstellung musste leider aufgrund der Serverprobleme der letzten Tage notgedrungen vorverlegt werden. Das neue Forum ist soweit voll funktionsfähig, allerdings sind noch nicht alle der gewohnten Funktionen vorhanden. Nach Möglichkeit werden wir sie in den nächsten Wochen nachrüsten. Dafür sollte es nun einige der Probleme lösen, die wir in den letzten Tagen, Wochen und Monaten hatten. Auch der Server ist nun potenter als bei unserem alten Hoster, wodurch wir nun langfristig den Tank mit Bytes vollgetankt haben.

    Anfangs mag die neue Boardsoftware etwas ungewohnt sein, aber man findet sich recht schnell ein. Wir wissen, dass ihr alle Gewohnheitstiere seid, aber gebt dem neuen Board eine Chance.
    Sollte etwas der neuen oder auch gewohnten Funktionen unklar sein, könnt ihr den "Wo issn da der Button zu"-Thread im Feedback nutzen. Bugs meldet ihr bitte im Bugtracker, es wird sicher welche geben die uns noch nicht aufgefallen sind. Ich werde das dann versuchen, halbwegs im Startbeitrag übersichtlich zu halten, was an Arbeit noch aussteht.

    Neu ist, dass die Boardsoftware deutlich besser für Mobiltelefone und diverse Endgeräte geeignet ist und nun auch im mobilen Style alle Funktionen verfügbar sind. Am Desktop findet ihr oben rechts sowohl den Umschalter zwischen hellem und dunklem Style. Am Handy ist der Hell-/Dunkelschalter am Ende der Seite. Damit sollte zukünftig jeder sein Board so konfigurieren können, wie es ihm am liebsten ist.


    Die restlichen Funktionen sollten eigentlich soweit wie gewohnt funktionieren. Einfach mal ein wenig damit spielen oder bei Unklarheiten im Thread nachfragen. Viel Spaß im ngb 2.0.

Go-Statistik Programm

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
@Brother John: Naja, in einem C-Struct habe ich noch nie Funktionen verwendet. Ich glaube das ist auch nicht möglich? Mehr ist das Struct ja eine Anordnung von "Daten" in einem frei definierbaren Datencontainer.... den man beliebig verwenden kann. Also ohne "privat" oder "public", weil wie du sagst, alles "public" ist. :)

Über Klassen spreche ich jetzt bewusst nicht...

Mal fern davon ab, meine Studienlektüre hat gerade die Einführung in "structs" (Strukturen), unions (Unions) und Aufzählungstypen in C++.
Und dann wohl noch einen Abstecher in "Modulare Programmierung" macht. - (worunter ich mir gerade noch nichts vorstellen kann)... vielleicht schon angewendet! :rolleyes: - I dont know. ;)
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #82
eine Gruppe sind also Spielsteine die "aneinander anliegen", wohlgemerkt vom gleichen Spieler, also mindestens 2 Steine die horizontal oder vertikal verbunden sind bilden dann eine Gruppe.
Stimmt.

Die Regeln, ob gegnerische Spielsteine "gefangen" sind macht dann das "Board".
Genau. Siehe bloody mess in applyTurn(Turn) und isValid(Turn).

Und ein "Spielstein" ist defacto über die Klasse "Coordinate.hpp" bzw. kann diese einen "Spielstein" haben ja/nein, deklariert.
Ja fast.
Die Coordinate gibt wirklich nur die Position an. Der Spielstein an sich ist in dem Repo "spielerlos". Die Zugehörigkeit zu einem Spieler wird erst durch Turn (hat Member m_color) bzw Group (hat auch Member m_color) bestimmt.


Also ohne jetzt den Code nochmal gelesen zu haben, würde ich aus deiner Beschreibung entnehmen das "isValid(Group)" überprüft, ob ein Spielstein "anliegt" und mit in die zu prüfende Gruppe dazu genommen werden kann?
Frage aber, kann ein Spielstein in zwei Gruppen parallel existieren? Oder fasst du Gruppen auch zusammen? (Ich hatte etwas von "Merge" in deinem Code gelesen).
Die Prüfung, zu welcher Gruppe ein Spielstein gehört oder ob er eine eigene bildet, passiert derzeit ebenfalls in applyTurn / isValid(Turn) durch Verwendung der Funktionen getNeighbours(Group).
Da muss noch überall aufgeräumt und ausgelagert werden. Darfst dich gerne dran versuchen ;)

Die Funktionen "m_x(Parameter)" und "m_y(Parameter)" werden "aufgerufen?
Ich habe das nie professionell gelernt. Daher hapert es auch bei mir mit den Begrifflichkeiten. Das sollte jetzt nicht so genau genommen werden. Es sieht aus wie ein Funktionsaufruf und funktioniert wie der Aufruf eines Construktors (der ja auch eine Funktion ist). Aber im Endeffekt hast du es ja verstanden - da passiert nichts weiter, als die Variablen auf einen gegebenen Wert zu setzen.

--- [2017-08-26 17:44 CEST] Automatisch zusammengeführter Beitrag ---

Naja, in einem C-Struct habe ich noch nie Funktionen verwendet. Ich glaube das ist auch nicht möglich?
Doch das ist möglich. Funktioniert 1:1 wie Klassen, nur das du standardmäßig ALLES von außerhalb des Structs erreichen kannst.
Du kannst aber auch ein "private" oder "protected" reinschreiben. Dann werden die Sachen auch versteckt.
Also mehr oder minder kaum ein Unterschied zu einer Klasse.
 

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
Doch das ist möglich. Funktioniert 1:1 wie Klassen, nur das du standardmäßig ALLES von außerhalb des Structs erreichen kannst.
Du kannst aber auch ein "private" oder "protected" reinschreiben. Dann werden die Sachen auch versteckt.
Also mehr oder minder kaum ein Unterschied zu einer Klasse.

Also in "C" (nicht C++ !) scheint das mit einem Struct und einer Function so nicht zu gehen (Weiterlesen). Aber ich kenn es auch nicht so. Das über was dort gesprochen wird sind "Function Pointer" (aber das hatten wir letztens auch irgendwo :D)

Die Prüfung, zu welcher Gruppe ein Spielstein gehört oder ob er eine eigene bildet, passiert derzeit ebenfalls in applyTurn / isValid(Turn) durch Verwendung der Funktionen getNeighbours(Group).
Da muss noch überall aufgeräumt und ausgelagert werden. Darfst dich gerne dran versuchen

Wie gesagt, wenn ich mal etwas mehr Ahnung hätte, ich steh noch so ziemlich am Anfang von all dem und was ich kann, da lacht ihr alle mal kurz. Ich werde da bestimmt nichts "optimieren" oder "aufräumen" (aufräumen von rechts nach links, oder in den Schrank oder unters Bett? :D )
Ich will damit sagen, ich hab wirklich noch keine Ahnung ;)

Ja fast.
Die Coordinate gibt wirklich nur die Position an. Der Spielstein an sich ist in dem Repo "spielerlos". Die Zugehörigkeit zu einem Spieler wird erst durch Turn (hat Member m_color) bzw Group (hat auch Member m_color) bestimmt.

Ich muss mir nochmal deinen Code anschauen, aber ich verstehe gerade nicht wo ein Gruppenloser Spielstein drin hängt, eine Gruppe wäre es ab 2 Spielsteinen, und im Turn hast du keine(?) Übersicht aller Spielfelder?
Wer verwaltet denn die "einzelnen" Spielsteine dann?

Fragen :D
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #84
Das über was dort gesprochen wird sind "Function Pointer" (aber das hatten wir letztens auch irgendwo :D)
Stimmt. Da habe ich eigentlich mal was zu gefragt - aber ich finde die Frage nicht wieder :D Ist aber auch ne ganze Weile her.

aber ich verstehe gerade nicht wo ein Gruppenloser Spielstein drin hängt, eine Gruppe wäre es ab 2 Spielsteinen
Ich habe eine Gruppe bereits als einen Spielstein definiert. Sobald welche verbunden sind, werden die der Gruppe hinzugefügt oder ggf. gemerged.
Coordinate gibt halt nur die Position an - noch nicht die Spielerfarbe. Das passiert erst in Verbindung mit einer der anderen beiden Klassen. Turn kennt die Farbe des Spielers, Group kennt die eigene Farbe und Board weiß, welche Farbe gerade dran ist / als nächstes zieht.


und im Turn hast du keine(?) Übersicht aller Spielfelder?
Turn beinhaltet
  • die Farbe des Spielers, der diesen Zug ausführen möchte,
  • die Coordinate, wo der Spielstein hingesetzt werden soll und
  • eine Id, damit die Turns später in einer Historie einfach unterschieden und in eine Reihenfolge gebracht werden können.

Wer verwaltet denn die "einzelnen" Spielsteine dann?
Board kennt alle bisherigen Züge (m_turnHistory), damit man im nächsten Schritt den Spielverlauf analysieren könnte (Statistik...).
Zudem kennt Board alle Gruppen, die aktuell auf dem Spielfeld liegen. Die Gruppen wiederrum kennen ihre Mitglieder, bzw. wo diese auf dem Spielfeld liegen.
Beim ausführen eines Zuges (applyTurn) wird geprüft, ob der Stein nicht doppelt vorhanden ist, zu welcher Gruppe er gehört und welche Auswirkungen er hat. (Dafür werden die meisten anderen Funktionen von Board und Member-Funktionen von Group verwendet.

Ist doch gut. So verstehst du den Code und vielleicht jemand anderes auch und man denkt nochmal über das bisher umgesetze nach.

--- [2017-08-26 19:11 CEST] Automatisch zusammengeführter Beitrag ---

Ich werde da bestimmt nichts "aufräumen" (aufräumen von rechts nach links, oder in den Schrank oder unters Bett? :D )
Ich habe zumindest mal ein wenig ausgelagert und so. Schon ein bisschen hübscher geworden.
Ich schau mal, ob ich da gleich noch etwas weiter mache :P

--- [2017-08-26 19:36 CEST] Automatisch zusammengeführter Beitrag ---

So. Etwas weiter aufgeräumt.
Vielleicht ist applyTurn und isValid(Turn) nun etwas einfacher zu lesen, wo nun die Funktionen ausgelagert sind und die einzelnen Funktionen deutlich kürzer geworden sind und fast nur noch aus Kommentaren bestehen.

BTW: Wie findet ihr die Kommentare? Sind die zu viel? Zu wenig? Ungenau? Alles genau richtig? Zu viele Fehler durch "schlechtes" Englisch?
 

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
@Roin:
Sorry das ich dir nicht gleich geantwortet habe, aber ich muss darüber doch mal etwas mehr nachdenken, bzw. wenn die Zeit dafür da ist bzw. der Kopf.
Zumindest verstehe ich nun deine "Strukturen", also deinen Aufbau, ein wenig mehr. Und kann mir jetzt schon besser vorstellen wie du "geplant" hast, damit zu arbeiten.

Wird natürlich nicht weniger Code, aber das hilft trotzdem schon ungemein zu kommentieren! ;)

Zu den Kommentaren, finde ich gar nicht zu viel eigentlich - für mich fast schon ideal, auch wenn es zu schreiben vielleicht, für einen selbst komisch ist:
if "X in Gruppe?"

---> X ist nicht / ist in Gruppe enthalten

Da würde ich mir sogar noch mehr Kommentar wünschen, also nicht was Schleifen angeht, aber alles was "logik" betrifft: X ist enthalten - prüfe ob X wirklich im Atari/gefangen ist, "prüfe" ob der Spieler dort setzen kann.... usw...

also alles, was Funktionen der Klassen oder aus anderen Headerfilles anspricht und somit nicht direkt in dem Code den man betrachtet sichtbar ist, was dieser tut - so fern nicht direkt selbst sprechend/erklärend.

Aber ein Kommentar tut in der Regel auch nicht weh, lieber mehr als zu wenig oder gar nicht, und gerade wenn man viele If, if else, else oder ähnliches hat, wäre das immer gut zu wissen wie eine Entscheidung weitergeht bzw. was erwartet ist bzw. wie die Ausgangslage ist.

Aber mach es einfach nach Bauchgefühl... alles worüber du mehr als 10 Sekunden nachdenken mußt, könnte einen Kommentar vertragen, oder ein "berüchtigter" One-Liner, der die ganze Arbeit macht. ;)

Es ist ja auch so, stell dir vor, du legst das Projekt mal für 3 Monate an die Seite, wenn du so rein gar nichts kommentiert hast, hast du definitiv Probleme dich in deine Logik wieder rein zu denken, und das fällt anderen dann noch schwerer. ;)
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #86
@theSplit: Die komplexen Funktionen wie applyTurn und isValid(Turn) (das sind wohl die komplexesten derzeit) haben auch deutlich mehr Kommentare als die kürzeren. Ich werde vielleicht die Tage nochmal rübergucken und weiter Kommentare verfassen, falls ich meine, irgendwo sind noch welche angebracht.
Dadurch, dass ich jetzt den dicken Code aber in einzelne Funktionen ausgelagert habe, sollte es insgesamt schon übersichtlicher geworden sein. Besonders da sowas wie applyTurn nun nicht mehr aus >100 Zeilen sondern nur noch aus ~13 besteht. Der Rest sind ja inzwischen Kommentare.
Aber gut - ein bisschen mehr die Logik-Schritte erklären. Werde ich versuchen ;)

Ich habe jetzt aber auch soweit ausgelagert, wie ich mir das irgendwie vorstellen konnte. Weiter vereinfachen geht von meiner Seite aus nicht mehr. Vielleicht findet da jemand anderes ja noch etwas.
 

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
@Roin: Ich schaue auch noch einmal darüber, aber erst morgen, nach dem Lernen.... nicht vorher, das ist fatal... vielleicht entdecke ich auch noch etwas. ;) :T

Und dann melde ich mich hier!
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #88
@Brother John: Schau dir doch bitte mal meine letzten Commits bezüglich Turn.hpp und Turn.cpp
Dort habe ich versucht die Klasse etwas schlanker zu fassen und auch bereits operatoren zu Implementieren, die man mit sicherheit später für die Statistic brauchen kann.

@all Dabei gilt zu beachten, dass ich den == Operator so geschrieben habe, dass die Farbe und das Zielfeld gleich sein müssen, nicht aber die Id. Die Id wird dafür aber bei den > und < Operatoren verwendet. Macht das für euch auch soweit Sinn?
 

Brother John

(schein)heilig
Veteran

Registriert
1 Aug. 2013
Beiträge
235
Structs in C haben tatsächlich keine Funktionen, keine Sichtbarkeit etc. Das sind alles Dinge aus der Objektorientierung. Genau deswegen sprech ichs an. Bei vorhandener C-Erfahrung ist es verführerisch, dem struct in C++ verkehrterweise dieselben Beschränkungen wie in C anzudichten. Es gibt in der C++-Welt nur tendenziell die Konvention, struct eher für dumme Datencontainer und class für komplexere Klassen zu verwenden.

Wie gesagt, wenn ich mal etwas mehr Ahnung hätte, ich steh noch so ziemlich am Anfang von all dem und was ich kann, da lacht ihr alle mal kurz.
Ach was! Keine Angst davor, schlechten Code zu produzieren! :) Lässt sich eh nicht vermeiden. Wenn du dir in einem Jahr deinen Code von heute anschaust und nicht ins Facepalmen verfällst, dann ist was schiefgelaufen.

@Roin
Turn gefällt mir so recht gut. Vielleicht von den Operatoren abgesehen, und zwar genau wegen deinem @all-Hinweis. So wie ich’s sehe gibt es zwei Arten von Verhältnis zwischen zwei Turns: inhaltliche Un-/Gleichheit und relative Position in der Zugabfolge. Jetzt ist aber das »gleich« in <= und >= (Abfolge) ein anderes »gleich« als das in == und != (Inhalt). Das zu vermischen, ist vermutlich keine gute Idee. Es gibt weniger Potenzial für Verwirrung, wenn du die eine Art Verhältnis über Operatoren implementierst und die andere über normale Funktionen. Ich würde es aber erst später umbauen, wenn klar wird, welche Vergleiche die öfter gebrauchten sind. Für die bieten sich die Operatoren an.

Detail: Das *inline* an den Operatoren ist überflüssig. Alle großen Compiler inlinen Funktionen nach ihren eigenen internen Regeln und kümmern sich kaum oder gar nicht darum, ob an einer Funktion *inline* dransteht. Zwingend hättest du das inline nur dranschreiben müssen, wenn du die Operatoren direkt im Header implementiert hättest. Das hat aber nichts mehr mit Inlining zu tun, sondern stellt nur sicher, dass die One Definition Rule erfüllt ist.

Auf GitHub hast du einen Pull-Request von »besc«, das bin ich. Was hab ich verbrochen?
  • Eine .gitignore-Datei, die das übliche lokale Zeugs ignoriert, das IDEs und die Versionsverwaltung gern erzeugen.
  • Ein rudimentäres CMake-Projekt. Das war schneller als Qt Creator mit dem Makefile zu verheiraten.
  • Und die Änderung, um die es mir eigentlich ging: Eine ganze Reihe von iterator-basierten For-Schleifen hab ich durch range-for ersetzt. Tun genau das gleiche, aber lesen sich viel hübscher. Einzige Einschränkung: range-for läuft immer über alle Elemente eines Containers. Deswegen sind die stellen, wo du von begin() + 1 an iterierst auch nicht umgestellt.
Nochwas in Board::getNeighbours():
Die zwei geschachtelten for-Schleifen benutzen denselben Variablennamen it. Das ist ziemlich gefährlich, weil der it der inneren Schleife den der äußeren Schleife versteckt. Ist bei allen Verwendungen von it in der inneren Schleife *wirklich* der innere it gemeint? Ich hab das jetzt nicht genau analysiert. Schau am besten nochmal drüber. Wenn an der Stelle alles geklärt ist, könntest du die beiden fors auch zu range-for umwandeln.
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #90
Detail: Das *inline* an den Operatoren ist überflüssig. Alle großen Compiler inlinen Funktionen nach ihren eigenen internen Regeln und kümmern sich kaum oder gar nicht darum, ob an einer Funktion *inline* dransteht. Zwingend hättest du das inline nur dranschreiben müssen, wenn du die Operatoren direkt im Header implementiert hättest. Das hat aber nichts mehr mit Inlining zu tun, sondern stellt nur sicher, dass die One Definition Rule erfüllt ist.
Das wusste ich nicht - ist aber denke ich, dann auch nicht weiter tragisch. Kommt bei Gelegenheit dann wieder raus.

  • Und die Änderung, um die es mir eigentlich ging: Eine ganze Reihe von iterator-basierten For-Schleifen hab ich durch range-for ersetzt. Tun genau das gleiche, aber lesen sich viel hübscher. Einzige Einschränkung: range-for läuft immer über alle Elemente eines Containers. Deswegen sind die stellen, wo du von begin() + 1 an iterierst auch nicht umgestellt.
Habe ich so noch nie verwendet. Fand meins bisher immer recht ansehnlich - aber stimmt. Deines ist dann doch etwas einfacher zu lesen. Aber einen Manko habe ich noch, welcher mir nicht gefällt:
auto - Ich mag das gar nicht. Das ist praktisch, wenn man schnell mal 4 Zeilen Code ausprobieren will. Aber ich möchte gerne sehen, welcher Dateityp dabei rumkommt. Es geht ja auch um Lesbarkeit und da finde ich es wichtig direkt zu sehen, welchen Dateityp denn nun die neue Variable hat. Natürlich ist das auch einleuchtend - doch ich mag es lieber, wenn es da auch explizit steht und sich nicht der Compiler Gedanken machen soll. Hast du das nicht? Ich finde, auto sollte verboten sein. :D

Nochwas in Board::getNeighbours():
Die zwei geschachtelten for-Schleifen benutzen denselben Variablennamen it. Das ist ziemlich gefährlich, weil der it der inneren Schleife den der äußeren Schleife versteckt. Ist bei allen Verwendungen von it in der inneren Schleife *wirklich* der innere it gemeint?
Gut gesehen. Habe ich übersehen. Es war zwar immer der innere it gemeint, doch ich habe es jetzt sowieso auf die tolle Range-For Schleife umgebaut.

Weißt du denn, wie es mit der Performance aussieht, wenn man die Range-For Schleife gegenüber der Iterator-Schleife nutzt? Oder ob es vielleicht sogar performancetechnisch auch sinnig ist, immer die Range-For zu nutzen und dann einfach eine Variable i zu inkrementieren, um zum Beispiel den ersten Eintrag zu überspringen?

--- [2017-08-27 01:07 CEST] Automatisch zusammengeführter Beitrag ---

Genau dazu ein wenig:
https://stackoverflow.com/questions/38349520/whats-the-difference-between-iterator-syntax-in-range-based-loops-for-stl-conta
 

Brother John

(schein)heilig
Veteran

Registriert
1 Aug. 2013
Beiträge
235
Roin schrieb:
Ich finde, auto sollte verboten sein.
Was!? Blasphemie! Auto ist das beste seit geschnitten Brot! … Naja, vielleicht nicht ganz, aber auf jeden Fall einer der dicken Vorteile von C++11. Wegen der Lesbarkeit. (Nicht nur, aber für mich ist das der wichtigste Punkt.) Meine persönliche Erfahrung sagt mir, dass oft genug der genaue Typ egal ist. Paradebeispiel Iterator. Ein Beispiel mit realistischen Typnamen aus meinem Code in der Arbeit könnte so aussehen:
[src=cpp]// ohne auto
const core::Uuid id = ...;
const std::unordered_map<core::Uuid, core::EntityWeakRef>::const_iterator found = m_signalLUT.find(id);

if (found != m_signalLUT.end()) {
const core::EntityWeakRef& signal = found->second;
// ...
}

// mit auto
const core::Uuid id = ...;
const auto found = m_signalLUT.find(id);

if (found != m_signalLUT.end()) {
const auto& signal = found->second;
// ...
}[/src]
Mit auto sehe ich auf einen Blick, was abgeht. Ohne auto muss ich mental diese Mega-Typangabe für den Iterator parsen, nur um dann festzustellen, dass mich der genaue Typ gar nicht interessiert. »Ist ein Iterator«, was wegen dem .find() offensichtlich genug ist, reicht vollkommen aus. Also vergesse ich den exakten Typen wieder und habe dabei vermutlich übersehen, dass *found* const deklariert ist.

Typen nicht zu sehen, ist auch nichts Neues. Das passiert ständig, ohne dass jemand Probleme damit hat. Z.B.:
[src=cpp]doWork(initStuff(foo), 5);[/src]
Hier siehst du nicht nur den Typen nicht, den initStuff() zurückgibt. Der Rückgabewert hat nicht mal mehr einen Namen!

Auto ist für mich ein extrem nützliches Tool, um uninteressante Typen nicht ausschreiben zu müssen. Frag aber bitte nicht, wo genau die Grenze zwischen interessant und uninteressant ist. Mehr als »Intuition« kann ich dazu nicht sagen. Und das wird auch in der C++-Community noch heißt diskutiert. Ein paar Beispiele:
GotW #94 Solution: AAA Style (Almost Always Auto)
Another Opinion on “Almost Always Auto”
Don't use auto unless you mean it (hochinteressante Reddit-Diskussion dazu)

Roin schrieb:
Weißt du denn, wie es mit der Performance aussieht, wenn man die Range-For Schleife gegenüber der Iterator-Schleife nutzt?
Performance ist dieselbe. Range-for ist eine Iteratorschleife mit netterer Syntax. Schau mal in dem Cppreference-Link aus dem letzten Post unter »Explanation«. Da siehst du das sehr deutlich.
 
Zuletzt bearbeitet:

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
So, fangen wir mal wegen der Kommentare, hatte ich ja gesagt ich mich darum bemühe diese sachlich und kritiisch zu hinterfragen:
Dabei habe ich versucht Dinge nicht zu hinterfragen, die für andere "selbstverständlich sein könnten", wobei so fast....

Turn.cpp

Zeile 6:
Wofür wird "Turn::instances" genutzt?
Die Erklärung dass die Variable als "static" deklariert worden ist zielführend, sagt aber nichts über den Nutzen. Auch spricht die Variable nicht direkt, bzw. verstehe ich den Zusammenhang nicht was diese später tun soll oder welche Wirkung sie hat.

Zeile 10-14:
Die Funktion "Turn:: Turn" hat als Beschreibung "Brief: Custom Constructor", sollte da nicht dann eher stehen "Advances to the next turn" oder so etwas? Knüpft aber auch direkt an das vorhergehende an, da dort die mysteriöse "instance" Variable genutzt wird.

Zeile 33 bzw. 30 - "Turn::setTarget":
hier werde ich nicht 100% schlau aus der Beschreibung, ist das "Target" in dem Fall das Feld auf den gesetzt wird?

Zeile 57: "Turn::getTarget":
gleiches Problem in der Beschreibung wie mit SetTarget, was ist "Target": "retrieves the players played field (target) in turn"?


Group.cpp

Zeile 36, "Group::getColor" : @brief Return the color of the group << Okay, aber hier wäre noch interessant zu erwähnen, das es sich um die "Spieler"farbe handelt, nicht nur um eine Farbe... diese Auslegung ist so nicht wirklich eindeutig.

Und vielleicht ebenfalls für "Group::setColor", Zeile 97 - das Wort "Spieler" mit einbringen.


Board.cpp
https://github.com/roin93/GoStatistics/blob/master/Board.cpp#L12: "InitPoints"
hier könnte man noch eine Beschreibung geben, das diese Funktion die Punkte der Spiele initialisiert, es ist zwar durch den Funktionsnamen recht verständlich, aber ein "Initializes the player points according to game rules" wäre nicht verkehrt.

Also bei "ApplyTurn" (erweitert und vereinfacht) - hast du ja schon sehr viel kommentiert. Das einzige was mir auffällt, kann man die Reihenfolge der Funktionen anders arrangieren?
Zum Beispiel, du rufst in "ApplyTurn" (erweitert) die Funktion "applyTurnToGroups" auf (Zeile 48). Diese wurde aber noch nicht beschrieben vorher.
Nachdem ich weiß was die Groups machen, kann ich mir auch denken was die Funktion macht, aber es ist trotz Kommentar in Zeile 47: "// Add the new stone to the Groups"
Vielleicht könnte man es so schreiben: Ein Stein wird einer existierenden Gruppe hinzugefügt ("oder eine neue Gruppe erstellt?")

Macht glaube ich Sinn was ich meine, da ich die Funktionsbeschreibung noch nicht gelesen habe, sehe ich die Funktion im Einsatz, aber hab keine Begrifflichkeiten dazu.
Deßhalb, wenn man die Reihenfolge etwas anpassen kann...

Zeile 64 und 65 in der Funktion:
"""
m_numberOfRemovedStonesLastTurn = 0;
m_removedStonesLastTurn.clear();
"""

Da bin ich raus, warum brauchen wir die Angabe was den "vorherigen" Zug angeht bzw. warum wird das gespeichert? Vielleicht wird das später noch klar, aber aktuell verstehe ich das große Bild dahinter nicht. Sind die Variablen für die Statistik? Etwas unklar.

Und nach Zeile 67:
if(!catchedGroups.empty())

schreibst du:
// Save the removed Stones to check if an Atari is given for the next Turn

Also, du speicherst die Steine die entfernt werden, um zu prüfen ob im "nächsten" (?) Zug ein Atari kommt?


Zeile 138 ist ein meiner Meinung nach etwas kryptisch, also fast ein schöner "OneLiner" ;)
if(not (*neighbourGroups.begin())->addMember(t.getTarget()))

Was evaluiert diese Funktion? - Sie gibt -1 zurück.
Ich weiß es steht in der Beschreibung der Funktion was das -1 bedeutet, aber ein Kommentar wäre für mich nicht verkehrt an genau so einer Stelle Code.
Und auch was der Rückgabewert dann innerhalb des "if" Blocks bedeutet. Der "else" Block des gleichen "if" fehlt auch eine kurze Notiz.

Zeile 178: "Board::changeColor" und Zeile 187 "Board::getColor" - vielleicht auch hier wieder "Spieler"farbe, anstelle nur das Wort "Farbe" zu benutzen?

Zeile 211, "Board::getCatchedGroups": Auch das "Brief:" Liefert die Gruppen die von einer "befreundeten Spieler"farbe umschlossen sind.
Das befreundet habe ich aus dem Param für Color, aber in der generell Beschreibung könnte man diesen "Twist" (das die Erklärung danach kommt durch den "color" Parameter) entfernen.

Zeile 239:
hasFreedoms = true; // Die Gruppe ist nicht gefangen?

Zeile 296, "Board::getMaxX"
Würdest du hier nicht ergänzen wollen, das die Funktion die "maximale Spielfeldbreite" (in der X-Achse) liefert?
Könnte man dann aber bei"getMaxY", "getMinX", getMinY" auch vielleicht entsprechend ergänzen - wie auch bei den "Setter" Funktionen.

Zeile 715: "Board::addPoints" - ich glaube diese und die darauf folgenden Funktionen sind später dazugekommen, oder? Hier ist noch nichts kommentiert. Wollte das nur anmerken.


So, ich glaube das reicht auch erst einmal, falls ich einige Anmerkungen zu oft wiederholt habe oder gebetsmühlenartig klinge, tuts mir leid. :D
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #93
@theSplit: Ach du s******. Das ist ein langer Post.
Ich bin gleich unterwegs, daher pflege ich die Wünsche erst demnächst ein. Aber ein paar Antworten sollst du kriegen.

Turn.cpp

Wofür wird "Turn::instances" genutzt?
Diese Variable ist für alle Instancen der Klasse Turn gleich. Durch sie zähle ich, wie viele Instancen zuvor generiert wurden und kann somit eine eindeutige Id vergeben. Durch diese lässt sich auch gleichzeitig der Zeitpunkt darstellen, wann ein Zug in der Vergangenheit gemacht wurde. Da bei so einem Spiel die reale Zeit irrelevant ist, reicht also eine Aussage á la "vor 5 Zügen ist das passiert."

Zeile 33 bzw. 30 - "Turn::setTarget":
hier werde ich nicht 100% schlau aus der Beschreibung, ist das "Target" in dem Fall das Feld auf den gesetzt wird?
Target beschreibt das Feld, auf das der Spielstein in diesem Zug gesetzt werden soll. Das Spielfeld ist ja durch die Coordinate beschrieben und wird dadurch eindeutig identifiziert.

Zeile 57: "Turn::getTarget":
gleiches Problem in der Beschreibung wie mit SetTarget, was ist "Target": "retrieves the players played field (target) in turn"?
Genau so ist es.

Group.cpp

Zeile 36, "Group::getColor" : @brief Return the color of the group << Okay, aber hier wäre noch interessant zu erwähnen, das es sich um die "Spieler"farbe handelt, nicht nur um eine Farbe... diese Auslegung ist so nicht wirklich eindeutig.
Namensklauberei ;) In diesem Zusammenhang gibt es nur Spielerfarben - aber ja, kann man etwas deutlicher machen.

Board.cpp


Also bei "ApplyTurn" (erweitert und vereinfacht) - hast du ja schon sehr viel kommentiert. Das einzige was mir auffällt, kann man die Reihenfolge der Funktionen anders arrangieren?
Reihenfolge in der Datei oder Reihenfolge in der Funktion? In der Funktion würde ich die ungern ändern, da dann der eigentliche Sinn der Funktion vermutlich nicht mehr erfüllt wird. In der Datei KANN man das ruhig anpassen.
Ich habe versucht die Funktionen in zwei Gruppen zu gruppieren. Einmal echte Board-Funktionen und Funktionen, die zwar irgendwas machen aber nicht unbedingt 100% zum Board gehören. Ansonsten habe ich versucht die Funktionen etwas alphabetisch zu sortieren, wobei ich sicherlich auch irgendwo durcheinander gekommen bin...

Zeile 64 und 65 in der Funktion:
"""
m_numberOfRemovedStonesLastTurn = 0;
m_removedStonesLastTurn.clear();
"""

Da bin ich raus, warum brauchen wir die Angabe was den "vorherigen" Zug angeht bzw. warum wird das gespeichert? Vielleicht wird das später noch klar, aber aktuell verstehe ich das große Bild dahinter nicht. Sind die Variablen für die Statistik? Etwas unklar.
In der Funktion isValid(Turn) müssen wir überprüfen, dass die Regel bezüglich Atari eingehalten wird.
Nochmal genau, was ich da prüfen möchte.
Es ist eine Situation entstanden, in der Schwarz einen Stein von Weiß fangen kann und umgekehrt. Schwarz hat nun den Stein gesetzt und damit den weißen Stein gefangen. Wenn nun Weiß den Stein auf das Feld setzt, wo der Stein zuvor gefangen wurde, dann würde er den schwarzen Stein fangen (Google einfach mal Atari Kö-Regel). Das könnte unendlich lange gemacht werden und die Punkte der Spieler steigen ebenso unendlich lange an. Damit dies nicht passiert, gibt es eine Spielregel, die dem zweiten (hier weiß) Spieler verbietet, in dem direkten Nachfolgezug auf das zuvor verlorene Feld zu setzen. Dafür muss bekannt sein, welcher Spielstein in dem Zug zuvor gefangen wurde. Da die gefangenen Steine aber aus den Datenstrukturen gelöscht werden, muss das entsprechend zwischengespeichert werden.


Zeile 138 ist ein meiner Meinung nach etwas kryptisch, also fast ein schöner "OneLiner" ;)
if(not (*neighbourGroups.begin())->addMember(t.getTarget()))
Es wird versucht der Spielstein, der durch t (Turn object) definiert ist, einer Gruppe hinzuzufügen.
Dies sollte normalerweise klappen. Wenn dies nicht klappt, dann wird -1 zurückgegeben und entsprechend der Zug nicht ausgeführt. Dies kann nur nicht klappen, wenn auf dem Feld bereits ein anderer Spielstein liegt.


Zeile 239:
hasFreedoms = true; // Die Gruppe ist nicht gefangen?
Genau. Wenn eine Gruppe mindestens eine Freiheit hat, ist sie nicht vollständig umschlossen ist somit nicht gefangen.


Zeile 715: "Board::addPoints" - ich glaube diese und die darauf folgenden Funktionen sind später dazugekommen, oder? Hier ist noch nichts kommentiert. Wollte das nur anmerken.
So weit habe ich schon seit einigen Tagen nicht mehr runtergescrollt :D :unknown:
 

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
@Roin: Die "Instance" Variable klingt sehr einleuchtet. Ich bin nur am Rätseln ob der Name passt, aber mit einem Kommentar sollte es dann auch mehr als eindeutig sein wozu die genutzt wird. Wie du sagst Wortklauberei ;)

Reihenfolge in der Datei oder Reihenfolge in der Funktion?

Ja, also schon noch die Anordnung bzw. Reihenfolge in der Datei. Wie gesagt, ich hatte beim lesen der "applyTurn" (erweitert) Funktion die anderen Funktionen die innerhalb des Anweisungsblocks verwendet werden, nicht richtig nachvollziehen können. Daher der Gedanke die Datei anders anzuordnen. Also das eine Definition einer Funktion, vor deren Verwendung kommt. Ich würde sagen so liest sich der Code dann auch einfacher.

Die "Kö"-Regel habe ich so nicht gekannt bisher, aber dann ist die Entscheidung, sich zu merken, welche Spielsteine in vorherigen Zug entfernt worden sind wohl mehr als richtig.

Danke für den Exkurs :T
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #95
@theSplit:
Ich habe mal ein paar weitere Kommentare geschrieben.
Die Reihenfolge der Funktionen in der Datei habe ich jetzt nicht geändert. Ich finde die alphabetische Reihenfolge der beiden Blöcke aktuell noch gut so. Aber das kann man sicher noch mal anpassen. Aber ich habe da gerade keine Muße zu :D Willst du das machen? :D
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #96
Könnte es sein, dass in der Logik noch die Möglichkeit fehlt, dass ein Spieler irgendwann aufgibt oder man auch zwischendurch mal passen darf? ("resign" und "pass" wären die entsprechenden Befehle, die eingeführt werden müssten)
 

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #97
Ich habe mir gerade ein paar Tutorials zu gtkmm durchgelesen (https://www.c-plusplus.net/forum/126450-full und die allgemeine Doku (ich versuche mich mit der 3.91.x) sowie die depricatedList und diesem zweiten Tutorial, da das erste echt alt ist...) und werde jetzt erstmal versuchen die nötigen Libs auf meinem System zu installieren und so.

Wenn das klappt, werde ich in einem gesonderten Branch mal die ersten kleinen Gehversuche mit gtkmm versuchen und mal schauen wie weit ich damit komme.

Ich habe daran gedacht, dass ich als Spielfeld eine gtkmm::Table nutze, die mit ein paar Unterschiedlichen Bildern gefüllt werden kann (
  • leeres Feld
  • leeres Randfeld oben
  • leeres Randfeld unten
  • leeres Randfeld links
  • leeres Randfeld rechts
  • leeres Randfeld (Ecke)
  • leeres Feld mit dickem Punkt
  • Feld mit weißem Stein
  • Feld mit schwarzem Stein
  • ...
)

Und alles weitere kann man dann ja irgendwie noch Anordnen.
Die Bilder habe ich derzeit noch nicht, aber ich fange erstmal an und dann schau ich mal weiter.

Mag da jemand helfen?

--- [2017-09-17 16:19 CEST] Automatisch zusammengeführter Beitrag ---

Schreib Makefiles bitte nicht selber!

Ich habe mal versucht das von dir angefertigte Makefile bei mir zu nutzen.

Kurz zu meinem aktuellen System: Ich habe MSYS2 mit MINGW_64 installiert.
Dazu via pacman make und cmake installiert.

Nun habe ich [src=bash]cmake .[/src] in dem Verzeichnis ausgeführt aber er erzeugt mir ganz viel Müll für Visual Studios statt einem einfachen Makefile, welches ich anschließend über [src=bash]make[/src] ausführen kann.
Kannst du mir sagen, wie ich da was anpassen muss?
Ich wollte von den ganzen Netbeans Files weg, weil die es ja nur unnötig schwieriger machen, denke ich. Aber ohne geht gerade einfach nichts...
 
Zuletzt bearbeitet:

Roin

Freier Denker

Registriert
22 Juli 2013
Beiträge
581
  • Thread Starter Thread Starter
  • #99
Möglicherweise habe ich da auch den falschen Namespace.
Das kommt im Tutorial (erster Link) vor. Das ist im Endeffekt ein Grid in dem man seine Elemente (Widgets) ausrichten kann. Sonst würde man da wohl horizontale Container und Vertikale Container nutzen. Die können ihre enthaltenen Elemente halt nur in eine Richtung nebeneinander / untereinander packen. Ich würde vermutlich eine Kombination aus den ganzen Dingern nutzen, um am Ende ein hübsches Fenster zu haben, in dem sich das meiste abspielt.
Aber leider habe ich jetzt erstmal das Problem, dass ich meinen Compiler umstellen wollte. Ich habe den zwar inzwischen am Laufen, aber das Projekt damit (vernünftig) zu kompilieren klappt leider nicht... Deswegen vor wenigen Minuten die Frage an Brother John mit dem Cmake...

--- [2017-09-17 16:28 CEST] Automatisch zusammengeführter Beitrag ---

Ist deprecated ab Version 3.4!
Ich habe auch das Tutorial von 2008 erst nur überflogen und bin noch nicht dazu gekommen auf die neueren Sachen umzusteigen.
Vermutlich wurde Table durch sowas wie GTK::Grid ersetzt.
 

theSplit

1998
Veteran Barkeeper

Registriert
3 Aug. 2014
Beiträge
28.560
@Roin: ich weiß nicht ob dir das weiterhilft - aber du kannst vielleicht auch ein "Grid" nehmen und dort Inhalte "positionieren", das wächst dynamisch mit, und du kannst Elemente, über ein Feld oder zwei oder X Breite und Höhe, Felder anordnen, je nachdem wie du das Layout haben willst.
Als Basis.

Der Table ist veraltet, siehe "Edit".

Ein Beispiel in "C" - auch wenn da viel anderer Code drin steckt und nicht nur "Layout":
Ein Grid an ein Fenster anknüpfen:
https://github.com/jrie/kisslib/blob/master/kiss-front.c#L476

Ein Element an:
x0, y0, Breite 10, Höhe 1: https://github.com/jrie/kisslib/blob/master/kiss-front.c#L512
x1, y1, Breite 9, Höhe 1: https://github.com/jrie/kisslib/blob/master/kiss-front.c#L591
usw... ;)
 
Zuletzt bearbeitet:
Oben