Eben, ich frage ob ich es aufzaehlen soll. Da mit einer Gegenfrage zu antworte, warum ich denn nichts aufzaehle, ist etwas albern. Hast doch grad selbst gesagt, ich soll einzelne Punkte rausgreifen und nicht alles aufzaehlen. Warum soll ich denn nicht alle aufzaehlen?
Also um mal einigiges herauszugreifen: Was mich am meisten stoert, sind die vielen Informationen die quer durch den Code verteilt sind. Mit A* selbst hat das aber ueberhaupt nichts zu tun. Beispielsweise ist an 5 Stellen die 10/14 als Distanzen definiert oder an 3 Stellen die Anzahl und Anordung der Nachbarknoten. Das kann keiner ueberschauen. Der Algorithmus, der die Karte visualisiert ist theoretisch auch unnoetig zerteilt. Einmal ist er in der Nodemap, die bei bestimmten Bedingungen eine Zeichen waehlt, aber gleichzeitig auch ueberall, wo "sign" gesetzt wird.
Dann finde ich diese getNeigbor Methode schlimm. Erstmal diese komische Pointersyntax, ist die wirklich noetig? Dann der im Prinzip gleiche Code untereinanderkopiert, waehrend andere Stellen auch noch von diesem Format abhaengig sind. Bei Aenderungen wieder kaum zu beherrschen. Die elegante Methode mit etwas Overhead waere hier sicher eine Array ueber die Beschreibung der 8 Nachbarn, z.b. OffsetX, OffsetY, Distance. Das saehe dann etwa so aus:
|
C-/C++-Quelltext
|
1
|
getNodeFromMap(x+neighbour_desc[neighbour_id].OffsetX, y+neighbour_desc[neighbour_id].OffsetY);
|
Sollte man irgendwann feststellen, das es hier zu einem Hotspot kommt, empfehle ich dann uebrigens folgendes:
|
C-/C++-Quelltext
|
1
2
3
4
5
6
7
8
9
10
11
12
13
|
// keeping all the information about current node to expand in members
m_CurPosX = x;
m_CurPosY = y;
m_CurNode = node;
// etc.
//"loop" over all neighbours
VisitNeighbour(x+1,y,Dist_Straight);
VisitNeighbour(x,y+1,Dist_Straight);
VisitNeighbour(x-1,y,Dist_Straight);
VisitNeighbour(x,y-1,Dist_Straight);
VisitNeighbour(x+1,y+1,Dist_Diagonal);
//etc..
|
Was VisitNeighbour macht, ist klar, oder?
Dritter Punkt: wenn das nicht nur ein Test, sondern ein ernsthafter Versuch eines performanten Algorithmus wird, sollte man sich gleich ueber die Datenstrukturen klar werden. Hier ist im Prinzip alle Information in der Node zusammengeschmissen. Das macht IMHO aber nicht viel Sinn. Zum Beispiel sind die Informationen, ob ein Feld begehbar sind, in der Node abgespeichert. Diese Daten sind aber nur Eingabedaten, die sollten woanders liegen. Die Wegsuche ist einer der Algorithmen die darauf zugreifen, aber nur lesend. Dann gibts Informationen zur Visualisierung, was mit dem Algorithmus auch nichts zu tun hat. Dann gibts noch einen Teil Daten, wo man sich nochmal fragen sollte, ob die wirklich noetig sind. Man muss bedenken, dass man erstmal den Speicher fuer die Node besorgen (auch in dem Sinne das er in den Cache geladen wird) muss und ihn dann initialisiert. Diese Datenstruktur wird danach aber gar nicht so oft gelesen, damit sich diese Ausgaben sich amortisieren koennten.
Und was jeder selber wissen muss, erinnert er sich nach 3 Monaten arbeiten an anderen Modulen noch was H und G bedeutet? Oder lieber gleich Namen wie "DistanceToHere" oder "EstimatedDistanceLeft" benutzen?