refactoring and comments

Makefile

@@ -1,12 +1,14 @@
 ARGS = -o fanorona -Wall -Wextra -pedantic -Werror
+SOURCE_CPP = source/Main.cpp source/Game.cpp source/String.cpp source/Vector.cpp source/Fields.cpp
+SOURCE_H = source/Game.h source/String.h source/Vector.h source/List.h source/Fields.h
 
 all: fanorona
 
 test: fanorona
 	socat TCP4-CONNECT:alpaga.hammerle.me:4455 EXEC:"./fanorona"
 	
-fanorona: source/Main.cpp source/Game.cpp source/Game.h source/String.cpp source/String.h source/Vector.cpp source/Vector.h source/List.h
-	g++ source/Main.cpp source/Game.cpp source/String.cpp source/Vector.cpp ${ARGS}
+fanorona: ${SOURCE_CPP} ${SOURCE_H}
+	g++ ${SOURCE_CPP} ${ARGS}
 	
 clean:
 	rm -f fanorona

source/Fields.cpp

@@ -0,0 +1,95 @@
+#include <iostream>
+
+#include "Fields.h"
+
+bool Fields::hasState(const Vector& v, State state) const {
+    return fields[v.x][v.y] == state;
+}
+
+Fields::State Fields::getState(const Vector& v) const {
+    return fields[v.x][v.y];
+}
+
+void Fields::setState(const Vector& v, State state) {
+    fields[v.x][v.y] = state;
+}
+
+void Fields::print(const Vector& active) {
+    String s;
+    toString(s, active);
+    std::cerr << s;
+}
+
+char Fields::getChar(int x, int y, const Vector& active) const {
+    if(active.compare(x, y)) {
+        return '*';
+    }
+    static const char map[] = {'#', 'O', '.'};
+    return map[fields[x][y]];
+}
+
+void Fields::toString(String& s, const Vector& active) const {
+    s.append("  0 1 2 3 4 5 6 7 8\n");
+    lineToString(s, 0, active);
+    s.append("  |\\|/|\\|/|\\|/|\\|/|\n");
+    lineToString(s, 1, active);
+    s.append("  |/|\\|/|\\|/|\\|/|\\|\n");
+    lineToString(s, 2, active);
+    s.append("  |\\|/|\\|/|\\|/|\\|/|\n");
+    lineToString(s, 3, active);
+    s.append("  |/|\\|/|\\|/|\\|/|\\|\n");
+    lineToString(s, 4, active);
+    s.append("\n");
+}
+
+void Fields::lineToString(String& s, int y, const Vector& active) const {
+    s.append(y + '0').append(' ');
+    for(int x = 0; x < 8; x++) {
+        s.append(getChar(x, y, active)).append('-');
+    }
+    s.append(getChar(8, y, active)).append("\n\r");
+}
+
+void Fields::consumeLine() const {
+    // skip until newline
+    while(std::cin.get() != '\n');
+    // skip any trailing line carriage
+    while(true) {
+        char c = std::cin.peek();
+        if(c != '\r') {
+            break;
+        }
+        std::cin.get();
+    }
+}
+
+void Fields::readLine(uint y) {
+    // skip line heading
+    std::cin.get();
+    std::cin.get();
+    std::cin.get();
+    for(uint x = 0; x < 9; x++) {
+        // read field state
+        char c = std::cin.get();
+        if(c == '2') {
+            fields[x][y] = BLACK;
+        } else if(c == '0') {
+            fields[x][y] = EMPTY;
+        } else if(c == '1') {
+            fields[x][y] = WHITE;
+        } else {
+            std::cerr << "game field parsing error\n";
+        }
+        // skip " - " until the next field (or the next line for x = 8)
+        std::cin.get();
+        std::cin.get();
+        std::cin.get();
+    }
+}
+
+void Fields::read() {
+    for(uint i = 0; i < 5; i++) {
+        consumeLine();
+        readLine(i);
+    }
+}

source/Fields.h

@@ -0,0 +1,33 @@
+#ifndef FIELDS_H
+#define FIELDS_H
+
+#include "Vector.h"
+#include "String.h"
+
+class Fields final {
+public:
+
+    enum State {
+        BLACK, WHITE, EMPTY
+    };
+
+    bool hasState(const Vector& v, State state) const;
+    State getState(const Vector& v) const;
+    void setState(const Vector& v, State state);
+
+    void print(const Vector& active);
+
+    void read();
+
+private:
+    char getChar(int x, int y, const Vector& active) const;
+    void toString(String& s, const Vector& active) const;
+    void lineToString(String& s, int y, const Vector& active) const;
+
+    void consumeLine() const;
+    void readLine(uint y);
+
+    State fields[9][5];
+};
+
+#endif

source/Game.cpp

@@ -1,4 +1,5 @@
 #include <iostream>
+#include <climits>
 
 #include "Game.h"
 #include "Types.h"
@@ -26,36 +27,6 @@ Game::Game() : active(-1, -1), mustTake(false) {
     }
 }
 
-char Game::getChar(int x, int y) const {
-    if(active.compare(x, y)) {
-        return '*';
-    }
-    static const char map[] = {'#', 'O', '.'};
-    return map[fields[x][y]];
-}
-
-void Game::toString(String& s) const {
-    s.append("  0 1 2 3 4 5 6 7 8\n");
-    lineToString(s, 0);
-    s.append("  |\\|/|\\|/|\\|/|\\|/|\n");
-    lineToString(s, 1);
-    s.append("  |/|\\|/|\\|/|\\|/|\\|\n");
-    lineToString(s, 2);
-    s.append("  |\\|/|\\|/|\\|/|\\|/|\n");
-    lineToString(s, 3);
-    s.append("  |/|\\|/|\\|/|\\|/|\\|\n");
-    lineToString(s, 4);
-    s.append("\n");
-}
-
-void Game::lineToString(String& s, int y) const {
-    s.append(y + '0').append(' ');
-    for(int x = 0; x < 8; x++) {
-        s.append(getChar(x, y)).append('-');
-    }
-    s.append(getChar(8, y)).append("\n\r");
-}
-
 void Game::readLine() {
     String line;
     while(true) {
@@ -73,8 +44,8 @@ void Game::readLine() {
 
 bool Game::parseLine(const String& line) {
     if(line == "   0   1   2   3   4   5   6   7   8") {
-        readFields();
-        printFields();
+        fields.read();
+        fields.print(active);
     } else if(line == "Please enter origin x-axis") {
         makeSelection();
     } else if(line == "Please enter destination x-axis") {
@@ -91,61 +62,11 @@ bool Game::parseLine(const String& line) {
     return false;
 }
 
-void Game::consumeLine() const {
-    // skip until newline
-    while(std::cin.get() != '\n');
-    // skip any trailing line carriage
-    while(true) {
-        char c = std::cin.peek();
-        if(c != '\r') {
-            break;
-        }
-        std::cin.get();
-    }
-}
-
-void Game::readLine(uint y) {
-    // skip line heading
-    std::cin.get();
-    std::cin.get();
-    std::cin.get();
-    for(uint x = 0; x < 9; x++) {
-        // read field state
-        char c = std::cin.get();
-        if(c == '2') {
-            fields[x][y] = BLACK;
-        } else if(c == '0') {
-            fields[x][y] = EMPTY;
-        } else if(c == '1') {
-            fields[x][y] = WHITE;
-        } else {
-            std::cerr << "game field parsing error\n";
-        }
-        // skip " - " until the next field (or the next line for x = 8)
-        std::cin.get();
-        std::cin.get();
-        std::cin.get();
-    }
-}
-
-void Game::readFields() {
-    for(uint i = 0; i < 5; i++) {
-        consumeLine();
-        readLine(i);
-    }
-}
-
-void Game::printFields() const {
-    String s;
-    toString(s);
-    std::cerr << s;
-}
-
 bool Game::isInRange(const Vector& v) const {
     return v.x >= 0 && v.x <= 8 && v.y >= 0 && v.y <= 4;
 }
 
-bool Game::areNeighbours(const Vector& from, const Vector& to) const {
+bool Game::areNeighbors(const Vector& from, const Vector& to) const {
     if(from == to) {
         return false;
     }
@@ -153,31 +74,35 @@ bool Game::areNeighbours(const Vector& from, const Vector& to) const {
     if(diff.x < -1 || diff.x > 1 || diff.y < -1 || diff.y > 1) {
         return false;
     }
+    // either left, right, upper or lower neighbor
     if(diff.x == 0 || diff.y == 0) {
         return true;
     }
-    return !((from.x & 1) ^ (from.y & 1));
+    // check if the field has diagonal connections (both coords are either even or uneven numbers)
+    return (from.x & 1) == (from.y & 1);
 }
 
-void Game::removeLine(const Vector& from, const Vector& to, FieldState state) {
+uint Game::removeLine(const Vector& from, const Vector& to, Fields::State state) {
     Vector current = to;
-    Vector diff = to - from;
+    Vector unit = to - from;
+    uint rank = 0;
     while(true) {
-        current += diff;
-        if(!isInRange(current) || !hasState(current, state)) {
-            return;
+        current += unit;
+        if(!isInRange(current) || !fields.hasState(current, state)) {
+            return rank;
         }
-        setState(current, EMPTY);
+        fields.setState(current, Fields::EMPTY);
+        rank++;
     }
 }
 
-uint Game::getRank(const Vector& from, const Vector& to, FieldState state) const {
+uint Game::getRank(const Vector& from, const Vector& to, Fields::State state) const {
     Vector current = to;
-    Vector diff = to - from;
+    Vector unit = to - from;
     uint rank = 0;
     while(true) {
-        current += diff;
-        if(!isInRange(current) || !hasState(current, state)) {
+        current += unit;
+        if(!isInRange(current) || !fields.hasState(current, state)) {
             return rank;
         }
         rank++;
@@ -185,28 +110,20 @@ uint Game::getRank(const Vector& from, const Vector& to, FieldState state) const
 }
 
 uint Game::getStoneTakes(const Vector& from, const Vector& to) const {
-    if(!isInRange(to) || !hasState(to, EMPTY) || !areNeighbours(from, to) || lastLocations.contains(to) || (to - from) == direction) {
+    // an invalid turn cannot take any stones
+    if(!isInRange(to) || !fields.hasState(to, Fields::EMPTY) || !areNeighbors(from, to) ||
+            lastLocations.contains(to) || (to - from) == direction) {
         return 0;
     }
-    FieldState enemy = (hasState(from, BLACK) ? WHITE : BLACK);
-    uint rankA = getRank(from, to, enemy);
-    uint rankB = getRank(to, from, enemy);
-    if(rankA == 0 && rankB == 0) {
+    // the turn must take stones
+    Fields::State enemy = (fields.hasState(from, Fields::BLACK) ? Fields::WHITE : Fields::BLACK);
+    if(getRank(from, to, enemy) == 0 && getRank(to, from, enemy) == 0) {
         return 0;
     }
-
+    // actually do the turn on a copied game, store the rank (= taken stones)
     Game copy = *this;
-    uint rank = 0;
-    if(rankA >= rankB) {
-        copy.removeLine(from, to, enemy);
-        rank = rankA;
-    } else {
-        copy.removeLine(to, from, enemy);
-        rank = rankB;
-    }
-    copy.lastLocations.add(from);
-    copy.move(from, to);
-
+    uint rank = copy.move(from, to);
+    // recursively check all neighbors on the copied game for another move, take the best move
     uint maxRank = 0;
     for(Vector& m : neighbours) {
         uint rank = copy.getStoneTakes(to, to + m);
@@ -214,73 +131,77 @@ uint Game::getStoneTakes(const Vector& from, const Vector& to) const {
             maxRank = rank;
         }
     }
-
     return rank + maxRank;
 }
 
-uint Game::getStoneFreedom(const Vector& from) const {
-    if(hasState(from, EMPTY)) {
+uint Game::getStoneFreedom(const Vector& from, Fields::State state) const {
+    // no freedom if the stone is not yours
+    if(!fields.hasState(from, state)) {
         return 0;
     }
+    // count directions the stone can move to
     uint counter = 0;
     for(const Vector& m : neighbours) {
         Vector to = from + m;
-        if(!isInRange(to) || !areNeighbours(from, to) || !hasState(to, EMPTY)) {
-            continue;
+        if(isInRange(to) && areNeighbors(from, to) && fields.hasState(to, Fields::EMPTY)) {
+            counter++;
         }
-        counter++;
     }
     return counter;
 }
 
-uint Game::getFreedom(FieldState state) const {
-    int sum = 0;
+uint Game::getFreedom(Fields::State state) const {
+    uint sum = 0;
     for(const Vector& v : fieldVectors) {
-        if(hasState(v, state)) {
-            sum += getStoneFreedom(v);
-        }
+        sum += getStoneFreedom(v, state);
     }
     return sum;
 }
 
-bool Game::hasState(const Vector& v, FieldState state) const {
-    return fields[v.x][v.y] == state;
-}
-
-Game::FieldState Game::getState(const Vector& v) const {
-    return fields[v.x][v.y];
-}
-
-void Game::setState(const Vector& v, FieldState state) {
-    fields[v.x][v.y] = state;
-}
-
-void Game::move(const Vector& from, const Vector& to) {
+uint Game::move(const Vector& from, const Vector& to) {
+    // remember direction to block the same move
     direction = to - from;
-    setState(to, getState(from));
-    setState(from, EMPTY);
+    // remember the old location to block moving to it again
+    lastLocations.add(from);
+    // actually from the stone
+    fields.setState(to, fields.getState(from));
+    fields.setState(from, Fields::EMPTY);
+    // get the enemy by inverting the goal state
+    Fields::State enemy = (fields.hasState(to, Fields::BLACK) ? Fields::WHITE : Fields::BLACK);
+    // compare quality of the two sides, take better side
+    uint rankA = getRank(from, to, enemy);
+    uint rankB = getRank(to, from, enemy);
+    if(rankA >= rankB) {
+        return removeLine(from, to, enemy);
+    }
+    return removeLine(to, from, enemy);
 }
 
 void Game::makeSelection() {
+    // new turn starts - reset previously blocked turns
     lastLocations.clear();
     direction.set(0, 0);
-
+    // remember if the following turn must take a stone
     mustTake = isTakingPossible();
-
+    // calculate rank for all possible selections, take best selection
     Vector selection(0, 0);
-    int maxRank = -100000;
+    int maxRank = INT_MIN;
     for(Vector& to : fieldVectors) {
-        if(!hasState(to, EMPTY)) {
+        if(!fields.hasState(to, Fields::EMPTY)) {
             continue;
         }
+        // scan neighbors of empty fields for white stones
         for(Vector& m : neighbours) {
             Vector from = to + m;
-            if(!isInRange(from) || !hasState(from, WHITE) || !areNeighbours(from, to)) {
+            // prevent invalid stone locations
+            if(!isInRange(from) || !fields.hasState(from, Fields::WHITE) || !areNeighbors(from, to)) {
                 continue;
             }
-            if(mustTake && getRank(from, to, BLACK) + getRank(to, from, BLACK) == 0) {
+            // prevent non taking turns if taking is a must
+            if(mustTake && getRank(from, to, Fields::BLACK) + getRank(to, from, Fields::BLACK) == 0) {
                 continue;
             }
+            // calculate rank and potentially store the new max value
             int rank = getQuality(from, to);
             if(rank > maxRank) {
                 maxRank = rank;
@@ -288,46 +209,57 @@ void Game::makeSelection() {
             }
         }
     }
+    // mark the stone as active
     active = selection;
-
+    // send the selection to the server
     std::cerr << "Selecting (" << selection.x << ", " << selection.y << ")\n";
-    String s;
-    toString(s);
-    std::cerr << s;
-
+    // print the selection and the current game field for logging purposes
+    fields.print(active);
     std::cout << selection.x << "\n" << selection.y << "\n";
 }
 
 void Game::makeMove() {
+    // check all neighbors of active selection and choose the move with the highest rank
     Vector maxTo(0, 0);
-    int maxRank = -100000;
+    int maxRank = INT_MIN;
     for(Vector& m : neighbours) {
         Vector to = active + m;
-        if(!isInRange(to) || !hasState(to, EMPTY) || lastLocations.contains(to) || !areNeighbours(active, to) || m == direction) {
+        // prevent invalid moves
+        if(!isInRange(to) || !fields.hasState(to, Fields::EMPTY) || lastLocations.contains(to) ||
+                !areNeighbors(active, to) || m == direction) {
             continue;
         }
+        // if this is not the first move or a must take move - prevent non taking move
         if(!lastLocations.isEmpty() || mustTake) {
-            int rank = getRank(active, to, BLACK) + getRank(to, active, BLACK);
+            int rank = getRank(active, to, Fields::BLACK) + getRank(to, active, Fields::BLACK);
             if(rank == 0) {
                 continue;
             }
         }
+        // calculate rank and potentially store the new max value
         int rank = getQuality(active, to);
         if(rank > maxRank) {
             maxRank = rank;
             maxTo = to;
         }
     }
+    // remember the old location to prevent moving to it
     lastLocations.add(active);
+    // remember the direction to prevent moving in it again
     direction = maxTo - active;
+    // mark the location as active
     active = maxTo;
+    // send the move to the server
     std::cerr << "Move to (" << maxTo.x << ", " << maxTo.y << ")\n";
+    // log the move
     std::cout << maxTo.x << "\n" << maxTo.y << "\n";
 }
 
 void Game::takeStone() {
-    uint rankA = getRank(active, active + direction, BLACK);
-    uint rankB = getRank(active + direction, active, BLACK);
+    // calculate the better taking side ...
+    uint rankA = getRank(active, active + direction, Fields::BLACK);
+    uint rankB = getRank(active + direction, active, Fields::BLACK);
+    // and send it to the server
     if(rankA >= rankB) {
         std::cout << "W\n";
     } else {
@@ -336,69 +268,77 @@ void Game::takeStone() {
 }
 
 int Game::getQuality(const Vector& from, const Vector& to) const {
+    // base quality is how much stones this turn can take (+ following moves)
     int quality = getStoneTakes(from, to);
-
+    // actually do the turn on a copy
     Game copy = *this;
     copy.move(from, to);
-    uint rankA = copy.getRank(from, to, BLACK);
-    uint rankB = copy.getRank(to, from, BLACK);
-    if(rankA >= rankB) {
-        copy.removeLine(from, to, BLACK);
-    } else {
-        copy.removeLine(to, from, BLACK);
-    }
+    // reset previous location and direction to simulate an opponent turn
     copy.lastLocations.clear();
     copy.direction.set(0, 0);
-
-    int whiteFreedom = copy.getFreedom(WHITE);
-    int blackFreedom = copy.getFreedom(BLACK);
-
-    int maxRank = -1;
-    for(Vector& to : fieldVectors) {
-        if(!copy.hasState(to, EMPTY)) {
-            continue;
-        }
-        for(Vector& m : neighbours) {
-            Vector from = to + m;
-            if(!copy.isInRange(from) || !copy.hasState(from, BLACK)) {
-                continue;
-            }
-            int rank = copy.getStoneTakes(from, to);
-            if(rank > maxRank) {
-                maxRank = rank;
-            }
-        }
-    }
-
+    // calculate white and black freedom on the potential new board
+    int whiteFreedom = copy.getFreedom(Fields::WHITE);
+    int blackFreedom = copy.getFreedom(Fields::BLACK);
+    // calculates the strongest opponent turn
+    uint maxRank = copy.getBestEnemyRank();
+    // opponent count is important in the endgame
     int black = countBlack();
     if(black >= 8) {
         black = 1;
     }
+    // finalize the quality formula
     return quality - maxRank * black + (whiteFreedom - blackFreedom) / 4;
 }
 
 int Game::countBlack() const {
-    int blackCounter = 0;
+    int counter = 0;
     for(Vector& v : fieldVectors) {
-        blackCounter += hasState(v, BLACK);
+        counter += fields.hasState(v, Fields::BLACK);
     }
-    return blackCounter;
+    return counter;
 }
 
 bool Game::isTakingPossible() const {
+    // scan all fields for possible taking moves
     for(Vector& to : fieldVectors) {
-        if(!hasState(to, EMPTY)) {
+        if(!fields.hasState(to, Fields::EMPTY)) {
             continue;
         }
+        // check if any white neighbored stone can move to the found empty stone
         for(Vector& m : neighbours) {
             Vector from = to + m;
-            if(!isInRange(from) || !areNeighbours(from, to) || !hasState(from, WHITE)) {
+            if(!isInRange(from) || !areNeighbors(from, to) || !fields.hasState(from, Fields::WHITE)) {
                 continue;
             }
-            if(getRank(from, to, BLACK) + getRank(to, from, BLACK) > 0) {
+            // return true if the white stone can take black stones in any direction
+            if(getRank(from, to, Fields::BLACK) + getRank(to, from, Fields::BLACK) > 0) {
                 return true;
             }
         }
     }
     return false;
+}
+
+uint Game::getBestEnemyRank() const {
+    uint maxRank = 0;
+    // scan all fields for best rank
+    for(Vector& to : fieldVectors) {
+        if(!fields.hasState(to, Fields::EMPTY)) {
+            continue;
+        }
+        // scan neighbors of found empty fields
+        for(Vector& m : neighbours) {
+            Vector from = to + m;
+            // prevent invalid moves
+            if(!isInRange(from) || !fields.hasState(from, Fields::BLACK)) {
+                continue;
+            }
+            // calculate and store the potentially better rank
+            uint rank = getStoneTakes(from, to);
+            if(rank > maxRank) {
+                maxRank = rank;
+            }
+        }
+    }
+    return maxRank;
 }

source/Game.h

@@ -7,47 +7,32 @@
 #include "Types.h"
 #include "Vector.h"
 #include "List.h"
+#include "Fields.h"
 
 class Game final {
 public:
-
-    enum FieldState {
-        BLACK, WHITE, EMPTY
-    };
-
     Game();
     void readLine();
 
 private:
     bool parseLine(const String& command);
-    
-    void consumeLine() const;
-    void readLine(uint y);
-    void readFields();
-    void printFields() const;
 
     bool isInRange(const Vector& v) const;
-    char getChar(int x, int y) const;
-    void toString(String& s) const;
-    void lineToString(String& s, int y) const;
     
-    bool areNeighbours(const Vector& from, const Vector& to) const;
+    bool areNeighbors(const Vector& from, const Vector& to) const;
     
-    void removeLine(const Vector& from, const Vector& to, FieldState state);
-    uint getRank(const Vector& from, const Vector& to, FieldState state) const;
+    uint removeLine(const Vector& from, const Vector& to, Fields::State state);
+    uint getRank(const Vector& from, const Vector& to, Fields::State state) const;
 
     uint getStoneTakes(const Vector& from, const Vector& to) const;
 
-    uint getStoneFreedom(const Vector& from) const;
-    uint getFreedom(FieldState state) const;
-
-    bool hasState(const Vector& v, FieldState state) const;
-    FieldState getState(const Vector& v) const;
-    void setState(const Vector& v, FieldState state);
+    uint getStoneFreedom(const Vector& from, Fields::State state) const;
+    uint getFreedom(Fields::State state) const;
     
-    void move(const Vector& from, const Vector& to);
+    uint move(const Vector& from, const Vector& to);
     
     void makeSelection();
+    
     void makeMove();
     void takeStone();
     
@@ -56,8 +41,10 @@ private:
     int countBlack() const;
     
     bool isTakingPossible() const;
-
-    FieldState fields[9][5];
+    
+    uint getBestEnemyRank() const;
+    
+    Fields fields;
     Vector active;
     Vector direction;
     bool mustTake;