Some comments added

src/net/hoo2/auth/dsproject/snake/Apple.java

@@ -1,7 +1,5 @@
 package net.hoo2.auth.dsproject.snake;
 
-import java.lang.Math;
-
 /**
  * @class Apple
  * @brief Represent an Apple in the Board.

src/net/hoo2/auth/dsproject/snake/Game.java

@@ -2,7 +2,7 @@ package net.hoo2.auth.dsproject.snake;
 
 /**
  * @mainpage
- * @title Snake game project. -- Part 1 --
+ * @title Snake game project. -- Part 2 --
  * 
  * This is the code documentation page of the Snake game project.
  * Listed are:
@@ -37,28 +37,41 @@ public class Game {
    /** @{ */
    private int round;         /**< The current round of the game */
    private Board board;       /**< A reference to board */
-   private ArrayList<Player>     players;  /**< A reference to players */
-   private Map<Integer, Integer> playingOrder;
+   private ArrayList<Player>     players;       /**< A reference to players */
+   private Map<Integer, Integer> playingOrder;  /**< A Map with {PlayerID, Dice-roll} pairs 
+                                      @note
+                                         This way of storing the playing order is unnecessary.
+                                         The old style was far more better. We had a turn variable
+                                         in each Player and we used to sort the players vector based on
+                                         that value. This made the rest of the program simpler, faster, easer.
+                                         We have adopt the above approach just because it is one of the
+                                         homework requirements.
+                                         */
    /** @} */
    
    /** @name private api */
    /** @{ */
 
    /**
-    * Search the players already in the players vector and compare their turn to play
+    * Search the players already in the pairs vector and compare their turn to play
     * with the result of a dice. If there is another one with the same dice result return true.
     * 
-    * @param turn       The dice result to check in order to find player's turn to play
-    * @param players    Reference to already register players
+    * @param roll       The dice result to check in order to find player's turn to play
+    * @param pairs      Reference to already register players and their dice result
     * @return           True if there is another player with the same dice result
     */
-   private boolean _search (int die, ArrayList<Integer[]> pairs) {
+   private boolean _search (int roll, ArrayList<Integer[]> pairs) {
       for (int i =0; i<pairs.size() ; ++i)
-         if (pairs.get(i)[1] == die)
+         if (pairs.get(i)[1] == roll)
             return true;
       return false;
    }
-   
+
+   /**
+    * Sort the pairs vector using second parameter witch represent the
+    * dice roll. We use bubblesort for the implementation.
+    * @param pairs   A vector with {PlayerId, dice-roll} pairs to sort
+    */
    private void _sort (ArrayList<Integer[]> pairs) {
       Integer[] temp;
       for (int i=pairs.size()-1 ; i>0 ; --i) {
@@ -71,7 +84,12 @@ public class Game {
          }
       }
    }
-   
+
+   /**
+    * Helper function to get a player from players vector using it's ID
+    * @param   playerId    The player's ID to seek
+    * @return  Reference to Player or null
+    */
    private Player _getPlayer(int playerId) {
       for (Player p : players) {
          if (p.getPlayerId() == playerId)
@@ -136,8 +154,9 @@ public class Game {
    void setPlayers(ArrayList<Player> players) {
       this.players = players;
    }
-   
+   /** Get reference to playingOrder Map */
    Map<Integer, Integer> getPlayingOrder () { return playingOrder; }
+   /** Set the playingOrder Map */
    void setPlayingOrder (Map<Integer, Integer> playingOrder) {
       this.playingOrder = playingOrder;
    }
@@ -159,7 +178,7 @@ public class Game {
    }
    
    /** 
-    * Register a player to the game
+    * Register a heuristic player to the game
     * @param playerId   The player ID to use
     * @param name       The player name to use
     * @return     The status of the operation
@@ -177,19 +196,23 @@ public class Game {
     * plays first which second and so on.
     */
    Map<Integer, Integer> setTurns (ArrayList<Player> players) {
-      int[]       pairData = new int[2];
-      Integer[]   PairData;
+      int[]       pairData = new int[2];  // We use plain int[] to create an Integer[]
+      Integer[]   PairData;     // The Integer[] to push to ArrayList<>          
       ArrayList<Integer[]>
-                  pairList = new ArrayList<>();
+                  pairList = new ArrayList<>(); // Use use ArrayList before Map in order to sort
 
       for (int d, i =0 ; i<players.size() ; ++i) {
          do
+            // Keep rolling the dice until we get a unique value
             d = players.get(i).dice ();
          while (_search (d, pairList));
+         // Make Integer[] pair
          pairData[0] = players.get(i).getPlayerId();
          pairData[1] = d;
-         PairData = Arrays.stream(pairData).boxed().toArray( Integer[]::new );
-         pairList.add(PairData);
+         PairData = Arrays.stream(pairData)
+                          .boxed()
+                          .toArray(Integer[]::new);
+         pairList.add(PairData); // Add to vector
       }
       // Sort playeingOrger
       _sort (pairList);
@@ -212,6 +235,7 @@ public class Game {
    /**
     * A game round. In each round every player plays when is his turn
     *
+    * @param   verbose  Flag to indicate how much we print to console
     * @return  The winner if we have one, or null
     */
    Player round (boolean verbose) {
@@ -301,6 +325,8 @@ public class Game {
             System.out.println("   " +p.getName() + ": " + p.getScore() +" points");
       }
 
+      // Print the extra statistics for the heuristic player only
+      // We use a little reflection for that
       for (Player p : game.getPlayers()) {
          if (p.getClass().getSimpleName().equals("HeuristicPlayer"))
             p.statistics(verbose, true);

src/net/hoo2/auth/dsproject/snake/HeuristicPlayer.java

@@ -2,26 +2,64 @@ package net.hoo2.auth.dsproject.snake;
 
 import java.util.*;
 
+/**
+ * @class HeuristicPlayer
+ * @brief Represent a Heuristic Player in the Game
+ * 
+ * The players are playing in a round-robin sequence and we keep track
+ * for each one of them their playing order, score and place on the board.
+ * This kind of player, is a cheater. He can control the dice. Not fair dude.
+ * 
+ * @author Christos Choutouridis AEM:8997
+ * @email  cchoutou@ece.auth.gr
+ */
 public class HeuristicPlayer
              extends Player {
 
-   private ArrayList<Integer[]> path;
-
+   /** @name Constructors */
+   /** @{ */
+   /** Default doing nothing constructor */
    public HeuristicPlayer() {
       super ();
       path = new ArrayList<Integer[]>();
    }
+   /**
+    * @brief   The main constructor
+    * 
+    * This creates a player for the game
+    * @param playerId   The player's to create
+    * @param name       The name of the player
+    * @param board      Reference to the board the player will play on.
+    */
    HeuristicPlayer (int playerId, String name, Board board) {
       super (playerId, name, board);
       path = new ArrayList<Integer[]>();
    }
+   /* @} */
 
-
+   /** @name Get/Set interface */
+   /** @{ */
+   ArrayList<Integer[]> getPath() { return path; }
+   void setPath (ArrayList<Integer[]> path) {
+      this.path = path;
+   }
+   /** @} */
+   
+   /**
+    * Override dice functionality for the player
+    * @return  As this is called from the game only to select playing order 
+    *          we cheat and return 1
+    */
    @Override
    int dice () {
       return 1;
    }
-
+   
+   /**
+    * Override get the next tile after the user's move
+    * @param tile    The initial tile
+    * @return  The tile after the move
+    */
    @Override
    int getNextMove (int tile) {
       Map<Integer, Double> moves = new HashMap<Integer, Double>();
@@ -29,6 +67,7 @@ public class HeuristicPlayer
       double ev  = Double.NEGATIVE_INFINITY;
       int   roll = 0;
 
+      // Evaluate each possible dice result and find the better one
       for (int r=1 ; r<=6 ; ++r) {
          moves.put (new Integer(r), evaluate (tile, r));
          if ((ev = moves.get(r)) > max) {
@@ -36,27 +75,37 @@ public class HeuristicPlayer
             roll = r;
          }
       }
+      // Do the move and get the move data
       Integer[] move_data = Arrays.stream(move (tile, roll, true))
                                   .boxed()
                                   .toArray(Integer[]::new);
+      // Store the move data
       path.add(move_data);
-      return tile + roll;
+      return tile + roll;  // return the new tile position
    }
-   
+
+   /**
+    * The Heuristic statistics version
+    * @param verbose    Flag to select the verbosity
+    * @param sum        Flag to select if we need to print a summarize of the user hystory
+    */
    @Override
    void statistics (boolean verbose, boolean sum) {
       if (sum) {
+         // If we run the summarize
          int nSnakes      =0;
          int nLadders     =0;
          int nRedApples   =0;
          int nBlackApples =0;
-         
+
+         // Calculate frequencies
          for (int i=0 ; i<path.size() ; ++i) {
             nSnakes += path.get(i)[MOVE_SNAKES_IDX];
             nLadders+= path.get(i)[MOVE_LADDERS_IDX];
             nRedApples   += path.get(i)[MOVE_RED_APPLES_IDX];
             nBlackApples += path.get(i)[MOVE_BLACK_APPLES_IDX];
          }
+         // Print the results
          System.out.println("");
          System.out.println("*** Statistics for " + name + " ***");
          System.out.println("   Number of Snake bites       : " + nSnakes);
@@ -66,15 +115,26 @@ public class HeuristicPlayer
          
       }
       else
+         // Call the base version
          super.statistics(verbose, sum);
    }
 
    
-   
+   /**
+    * The main evaluation function
+    * @param tile    The current tile of the player
+    * @param roll    the roll to check
+    * @return        The evaluation of the roll
+    */
    private double evaluate (int tile, int roll) {
       int[] check = new int[MOVE_DATA_SIZE]; 
       check = move(tile, roll, false);
       
       return 0.65*check[MOVE_STEPS_IDX] + 0.35*check[MOVE_POINTS_IDX];
    }
+
+   /** @name Data members package access only */
+   /** @{ */
+   private ArrayList<Integer[]> path;  /**< Players history as required */
+   /** @} */
 }

src/net/hoo2/auth/dsproject/snake/Player.java

@@ -1,6 +1,7 @@
 package net.hoo2.auth.dsproject.snake;
 
 import java.util.Arrays;
+import java.lang.Math;
 
 /**
  * @class Player
@@ -13,16 +14,17 @@ import java.util.Arrays;
  * @email  cchoutou@ece.auth.gr
  */
 public class Player {
-   static final int MOVE_DATA_SIZE        = 9;
-   static final int MOVE_TILE_IDX         = 0;
-   static final int MOVE_INITTILE_IDX     = 1;
-   static final int MOVE_STEPS_IDX        = 2;
-   static final int MOVE_ROLL_IDX         = 3;
-   static final int MOVE_POINTS_IDX       = 4;
-   static final int MOVE_SNAKES_IDX       = 5;  // Always <= 1
-   static final int MOVE_LADDERS_IDX      = 6;  // Always <= 1
-   static final int MOVE_RED_APPLES_IDX   = 7;  // Always <= 1
-   static final int MOVE_BLACK_APPLES_IDX = 8;  // Always <= 1
+   /** Helper variables to keep track of the move() return values @see move() */
+   static final int MOVE_DATA_SIZE        = 9;  /**< The move return data array size */
+   static final int MOVE_TILE_IDX         = 0;  /**< The index of tile */
+   static final int MOVE_INITTILE_IDX     = 1;  /**< The index of  init tile */
+   static final int MOVE_STEPS_IDX        = 2;  /**< The index of steps */
+   static final int MOVE_ROLL_IDX         = 3;  /**< The index of roll */
+   static final int MOVE_POINTS_IDX       = 4;  /**< The index of  points */
+   static final int MOVE_SNAKES_IDX       = 5;  /**< The index for number of snakes (Always <= 1) */
+   static final int MOVE_LADDERS_IDX      = 6;  /**< The index for number of ladders (Always <= 1) */
+   static final int MOVE_RED_APPLES_IDX   = 7;  /**< The index for number of red apples (Always <= 1) */
+   static final int MOVE_BLACK_APPLES_IDX = 8;  /**< The index for number of black apples (Always <= 1) */
 
    /** @name Constructors */
    /** @{ */
@@ -97,17 +99,26 @@ public class Player {
       return (int)(1 + Math.random()*5);
    }
    
-   // Liskov substitution principle
+   /**
+    * Get the next tile after the user's move
+    * @param tile    The initial tile
+    * @return  The tile after the move
+    * @note
+    *    We add this move() wrapper in order to provide polymorphism to
+    *    Player class hierarchy and to be sure that we are not braking the
+    *    Liskov substitution principle
+    *    @see https://en.wikipedia.org/wiki/Liskov_substitution_principle
+    */
    int getNextMove (int tile) {
       return move (tile, dice(), true)[MOVE_TILE_IDX];
    }
   
    /**
     * @brief   Move functionality
-    * This function prints to stdout various logs about user interaction with elements
     * 
     * @param tile    The initial tile of the player
-    * @param die     The die to play
+    * @param roll    The dice roll to play
+    * @param run     Flag to indicate if we fake the move(dry run) or we do make the move.
     * @return
     *    int[MOVE_TILE_IDX         (0)]   tile after move
     *    int[MOVE_INITTILE_IDX     (1)]   tile before move
@@ -118,6 +129,17 @@ public class Player {
     *    int[MOVE_LADDERS_IDX      (6)]   number of ladders used
     *    int[MOVE_RED_APPLES_IDX   (7)]   number of red apples eaten
     *    int[MOVE_BLACK_APPLES_IDX (8)]   number of black apples eaten
+    * @note
+    *    Probably a mutable class like: <pre>
+    *    class MoveData {
+    *       int tile;
+    *       int initTile;
+    *       ...
+    *    }
+    *    </pre>
+    *    for returning value would be better, less error prone, cleaner, etc...
+    *    We could also had members like: <pre> MoveData previous; </pre> to help us further.
+    *    We kept this representation just because it was a requirement.
     */
    int [] move (int tile, int roll, boolean run) {
       int t;
@@ -154,6 +176,7 @@ public class Player {
 
       dryMove[MOVE_TILE_IDX] = tile;
       dryMove[MOVE_STEPS_IDX]= tile - dryMove[MOVE_INITTILE_IDX];
+      // Check if we do run the move
       if (run) {
          lastMove = dryMove.clone();
          this.tile = lastMove[MOVE_TILE_IDX];
@@ -162,7 +185,16 @@ public class Player {
       return dryMove;
    }
    
-   //Liskov substitution principle
+   /**
+    * The base statistics version
+    * @param verbose    Flag to select the verbosity
+    * @param sum        Flag to select if we need to print a summarize (not used here)
+    * @note
+    *    We added this function because:
+    *    1) we need to keep the "Is a" relationship (Liskov substitution principle)
+    *    2) It help us get rid of the move() console output code
+    *    3) makes the code smaller
+    */
    void statistics (boolean verbose, boolean sum) {
       int begin = lastMove[MOVE_INITTILE_IDX];
       int roll  = lastMove[MOVE_ROLL_IDX];
@@ -188,8 +220,8 @@ public class Player {
    int    score;      /**< Player's score */
    Board  board;      /**< Reference to current board */
    int    tile;       /**< Player's tile location */
-   int[]  lastMove;
-   private int [] dryMove;
+   int[]  lastMove;   /**< move() return data for statistics. These are only valid after a true move */
+   private int [] dryMove;    /**< Fake (dry run) move return buffer */
    /** @} */
 }