bool + log ad hoc --> Result<_,_> & refactoring

1 files changed, 218 insertions, 247 deletions

diff --git a/src/game.rs b/src/game.rs
index 057a307..1d61732 100644
--- a/src/game.rs
+++ b/src/game.rs
@@ -177,6 +177,14 @@ impl GameState {
         }
     }
 
+    fn within_bounds(&self, pos: &Position) -> bool {
+        if (pos.x < self.size) && (pos.y < self.size) {
+            true
+        } else {
+            false
+        }
+    }
+
     fn pos_to_idx(&self, pos: &Position) -> usize {
         let y: usize = pos.y as usize;
         let x: usize = pos.x as usize;
@@ -187,8 +195,43 @@ impl GameState {
         self.board.get(self.pos_to_idx(pos))
     }
 
-    // Unconditional and so could lead to invalid state, but private
-    fn place_stone(&self, player: Player, pos: &Position, stone: Stone) -> GameState {
+    fn is_legal_place(&self, player: Player, pos: &Position, stone: Stone) -> Result<(), String> {
+        /* In order to legally place a piece:
+        1. The desired square must be empty
+        2. The player must have sufficient pieces
+         */
+        if self.within_bounds(pos) {
+            if let Some(stack) = &self.board.get(self.pos_to_idx(pos)) {
+                if stack.len() > 0 {
+                    return Err(format!("{} is already occupied.", pos));
+                } else if self.remaining_pieces(player, stone) == 0 {
+                    return Err(format!(
+                        "{} has no more remaining {} pieces.",
+                        player, stone
+                    ));
+                } else {
+                    return Ok(());
+                }
+            } else {
+                return Err(format!(
+                    "Internal error: lookup for {} failed in is_legal_place.",
+                    pos
+                ));
+            }
+        } else {
+            return Err(format!("Position {} is not within bounds.", pos));
+        }
+    }
+
+    fn place_stone(
+        &self,
+        player: Player,
+        pos: &Position,
+        stone: Stone,
+    ) -> Result<GameState, String> {
+        if let Err(e) = self.is_legal_place(player, pos, stone) {
+            return Err(e);
+        }
         // Place stone
         let mut copy = self.copy();
         let idx = self.pos_to_idx(&pos);
@@ -213,10 +256,120 @@ impl GameState {
                 }
             }
         }
-        copy
+        Ok(copy)
     }
 
-    fn move_stack(&self, pos: &Position, direction: Direction, drops: &Vec<u8>) -> GameState {
+    fn is_legal_move(
+        &self,
+        player: Player,
+        pos: &Position,
+        direction: Direction,
+        drops: &Vec<u8>,
+    ) -> Result<(), String> {
+        if self.within_bounds(pos) {
+            /* Rules for moving a stack:
+            0. There are stones
+            1. Top stone belongs to player
+            2. Zero or One stones dropped on starting square
+            3. Total number of stones moved does not exceed the carry capacity
+            4. Direction does not contain a capstone
+            5. Wall may only appear on last spot if it's capstone alone that covers
+            6. All stones are used up before then end of the board is met
+             */
+            if let Some(stack) = &self.board.get(self.pos_to_idx(pos)) {
+                if stack.len() == 0 {
+                    return Err(format!("{} has no stones to move.", pos));
+                } else if drops.len() == 0 {
+                    return Err(String::from(
+                        "A drop sequence for must be specified for a move.",
+                    ));
+                } else if stack[0].player != player {
+                    return Err(format!(
+                        "{} may not move the stack at {} as it belongs to {}.",
+                        player, pos, stack[0].player
+                    ));
+                } else if drops[0] > 1 {
+                    return Err(format!(
+                        "A move may only drop 0 or 1 stones at it's origin square."
+                    ));
+                // Do the sum in u32 just in case ?
+                } else if drops.iter().map(|&d| d as u32).sum::<u32>() > self.size as u32 {
+                    return Err(format!(
+                        "A move may not exceed the carry limit of {} stones.",
+                        self.size
+                    ));
+                }
+                let mut steps: usize = drops.len() - 1;
+                let cap: bool = stack[0].stone == Stone::Capstone;
+                let mut pos_new = Position { x: pos.x, y: pos.y };
+
+                while steps > 0 {
+                    steps -= 1;
+                    if {
+                        match direction {
+                            Direction::Up => pos_new.y + 1 >= self.size,
+                            Direction::Down => pos_new.y == 0,
+                            Direction::Left => pos_new.x + 1 >= self.size,
+                            Direction::Right => pos_new.x == 0,
+                        }
+                    } {
+                        return Err(String::from("A move may not extend past the board."));
+                    } else {
+                        match direction {
+                            Direction::Up => pos_new.y += 1,
+                            Direction::Down => pos_new.y -= 1,
+                            Direction::Left => pos_new.x += 1,
+                            Direction::Right => pos_new.x -= 1,
+                        }
+                        if let Some(stack) = &self.board.get(self.pos_to_idx(&pos_new)) {
+                            if stack.len() > 0 {
+                                match stack[0].stone {
+                                    Stone::Capstone => {
+                                        return Err(String::from(
+                                            "A move may not cover a capstone.",
+                                        ));
+                                    }
+                                    Stone::Standing => {
+                                        if (steps > 1) || (!cap) {
+                                            return Err(String::from(
+                                                "A move may not cover a standing stone.",
+                                            ));
+                                        }
+                                    }
+                                    Stone::Flat => (),
+                                }
+                            }
+                        } else {
+                            return Err(format!(
+                                "Internal error: lookup for {} failed in is_legal_move (1).",
+                                pos_new
+                            ));
+                        }
+                    }
+                }
+                return Ok(());
+            } else {
+                return Err(format!(
+                    "Internal error: lookup for {} failed in is_legal_move (2).",
+                    pos
+                ));
+            }
+        } else {
+            return Err(format!("Position {} is not within bounds.", pos));
+        }
+    }
+
+    fn move_stack(
+        &self,
+        player: Player,
+        pos: &Position,
+        direction: Direction,
+        drops: &Vec<u8>,
+    ) -> Result<GameState, String> {
+        if let Err(e) = self.is_legal_move(player, pos, direction, drops) {
+            return Err(e);
+        }
+
         let mut copy = self.copy();
         let mut pos_new = Position { x: pos.x, y: pos.y };
         let stack: &Stack = &self.board[self.pos_to_idx(pos)];
@@ -235,7 +388,7 @@ impl GameState {
                 Direction::Right => pos_new.x += 1,
             };
         }
-        copy
+        Ok(copy)
     }
 }
 
@@ -245,12 +398,7 @@ enum TurnOrder {
     Normal,
 }
 
-// TODO: Redo this logging story with Result<...,String> instead
-
-pub struct Game<L>
-where
-    L: Fn(String),
-{
+pub struct Game {
     size: u8,
     current_player: Player,
     turn_order: TurnOrder,
@@ -258,38 +406,29 @@ where
     black_player_name: String,
     actions: Vec<Action>,
     states: Vec<GameState>,
-    log: L,
 }
 
-impl<L: Fn(String)> Game<L> {
-    pub fn new(size: u8, white_player_name: &str, black_player_name: &str, log: L) -> Game<L> {
-        let size = if (size <= 3) || (size >= 8) {
-            (log)(format!(
-                "Warning: the requested game size of {}x{} is not supported, defaulting to 5x5.",
-                size, size
-            ));
-            5
+impl Game {
+    pub fn new(size: u8, white_player_name: &str, black_player_name: &str) -> (Game, String) {
+        let (size, warning) = if (size <= 3) || (size >= 8) {
+            (5, format!("Warning: the requested game size of {}x{} is not supported, defaulting to 5x5.",
+                    size, size))
         } else {
-            size
+            (size, String::new())
         };
-        Game {
-            size: size,
-            white_player_name: white_player_name.to_string(),
-            black_player_name: black_player_name.to_string(),
-            states: vec![GameState::new(size)],
-            actions: Vec::new(),
-            current_player: Player::Black,
-            turn_order: TurnOrder::BlackPlacesWhite,
-            log: log,
-        }
-    }
 
-    fn within_bounds(&self, pos: &Position) -> bool {
-        if (pos.x < self.size) && (pos.y < self.size) {
-            true
-        } else {
-            false
-        }
+        (
+            Game {
+                size: size,
+                white_player_name: white_player_name.to_string(),
+                black_player_name: black_player_name.to_string(),
+                states: vec![GameState::new(size)],
+                actions: Vec::new(),
+                current_player: Player::Black,
+                turn_order: TurnOrder::BlackPlacesWhite,
+            },
+            warning,
+        )
     }
 
     // Relying on only ::new(...) being used to make instances
@@ -299,11 +438,7 @@ impl<L: Fn(String)> Game<L> {
     }
 
     pub fn query_square(&self, pos: &Position) -> Option<&Stack> {
-        if self.within_bounds(pos) {
-            self.last_state().query_pos(pos)
-        } else {
-            None
-        }
+        self.last_state().query_pos(pos)
     }
 
     pub fn query_pieces(&self, player: Player, stone: Stone) -> u8 {
@@ -314,9 +449,9 @@ impl<L: Fn(String)> Game<L> {
         self.current_player
     }
 
-    pub fn query_action(&self, turn: u16) -> Option<&Action> {
-        self.actions.get(turn as usize)
-    }
+    // pub fn query_action(&self, turn: u16) -> Option<&Action> {
+    //     self.actions.get(turn as usize)
+    // }
 
     pub fn get_size(&self) -> u8 {
         self.size
@@ -340,177 +475,24 @@ impl<L: Fn(String)> Game<L> {
         result
     }
 
-    fn is_legal_place(&self, player: &Player, pos: &Position, stone: &Stone) -> bool {
-        if let Some(state) = self.states.last() {
-            /* In order to legally place a piece:
-            1. The desired square must be empty
-            2. The player must have sufficient pieces
-             */
-            if self.within_bounds(pos) {
-                if let Some(stack) = &state.board.get(state.pos_to_idx(pos)) {
-                    if stack.len() > 0 {
-                        (self.log)(format!("{} is already occupied.", pos));
-                        false
-                    } else if state.remaining_pieces(*player, *stone) == 0 {
-                        (self.log)(format!(
-                            "{} has no more remaining {} pieces.",
-                            player, stone
-                        ));
-                        false
-                    } else {
-                        true
-                    }
-                } else {
-                    (self.log)(format!(
-                        "Internal error: lookup for {} failed in is_legal_place.",
-                        pos
-                    ));
-                    false
-                }
-            } else {
-                (self.log)(format!("Position {} is not within bounds.", pos));
-                false
-            }
-        } else {
-            (self.log)(format!(
-                "Internal error: cannot check action with no game state."
-            ));
-            false
+    pub fn perform_action(&mut self, act: Action) -> Result<(), String> {
+        let maybe_state = self.states.last();
+        if maybe_state.is_none() {
+            return Err(String::from("Internal error: cannot find last game state"));
         }
-    }
-
-    fn is_legal_move(
-        &self,
-        player: &Player,
-        pos: &Position,
-        direction: &Direction,
-        drops: &Vec<u8>,
-    ) -> bool {
-        if let Some(state) = self.states.last() {
-            if self.within_bounds(pos) {
-                /* Rules for moving a stack:
-                0. There are stones
-                1. Top stone belongs to player
-                2. Zero or One stones dropped on starting square
-                3. Total number of stones moved does not exceed the carry capacity
-                4. Direction does not contain a capstone
-                5. Wall may only appear on last spot if it's capstone alone that covers
-                6. All stones are used up before then end of the board is met
-                 */
-                if let Some(stack) = &state.board.get(state.pos_to_idx(pos)) {
-                    if stack.len() == 0 {
-                        (self.log)(format!("{} has no stones to move.", pos));
-                        return false;
-                    } else if drops.len() == 0 {
-                        (self.log)(String::from(
-                            "A drop sequence for must be specified for a move.",
-                        ));
-                        return false;
-                    } else if stack[0].player != *player {
-                        (self.log)(format!(
-                            "{} may not move the stack at {} as it belongs to {}.",
-                            player, pos, stack[0].player
-                        ));
-                        return false;
-                    } else if drops[0] > 1 {
-                        (self.log)(format!(
-                            "A move may only drop 0 or 1 stones at it's origin square."
-                        ));
-                        return false;
-                    // Do the sum in u32 just in case ?
-                    } else if drops.iter().map(|&d| d as u32).sum::<u32>() > state.size as u32 {
-                        (self.log)(format!(
-                            "A move may not exceed the carry limit of {} stones.",
-                            state.size
-                        ));
-                        return false;
-                    }
-                    let mut steps: usize = drops.len() - 1;
-                    let cap: bool = stack[0].stone == Stone::Capstone;
-                    let mut pos_new = Position { x: pos.x, y: pos.y };
-
-                    while steps > 0 {
-                        steps -= 1;
-                        if {
-                            match direction {
-                                Direction::Up => pos_new.y + 1 >= state.size,
-                                Direction::Down => pos_new.y == 0,
-                                Direction::Left => pos_new.x + 1 >= state.size,
-                                Direction::Right => pos_new.x == 0,
-                            }
-                        } {
-                            (self.log)(String::from("A move may not extend past the board."));
-                            return false;
-                        } else {
-                            match direction {
-                                Direction::Up => pos_new.y += 1,
-                                Direction::Down => pos_new.y -= 1,
-                                Direction::Left => pos_new.x += 1,
-                                Direction::Right => pos_new.x -= 1,
-                            }
-                            if let Some(stack) = &state.board.get(state.pos_to_idx(&pos_new)) {
-                                if stack.len() > 0 {
-                                    match stack[0].stone {
-                                        Stone::Capstone => {
-                                            (self.log)(String::from(
-                                                "A move may not cover a capstone.",
-                                            ));
-                                            return false;
-                                        }
-                                        Stone::Standing => {
-                                            if (steps > 1) || (!cap) {
-                                                (self.log)(String::from(
-                                                    "A move may not cover a standing stone.",
-                                                ));
-                                                return false;
-                                            }
-                                        }
-                                        Stone::Flat => (),
-                                    }
-                                }
-                            } else {
-                                (self.log)(format!(
-                                    "Internal error: lookup for {} failed in is_legal_move (1).",
-                                    pos_new
-                                ));
-                                return false;
-                            }
-                        }
-                    }
-                    return true;
-                } else {
-                    (self.log)(format!(
-                        "Internal error: lookup for {} failed in is_legal_move (2).",
-                        pos
-                    ));
-                    return false;
-                }
-            } else {
-                (self.log)(format!("Position {} is not within bounds.", pos));
-                false
-            }
-        } else {
-            (self.log)(String::from(
-                "Internal error: cannot check action with no game state.",
-            ));
-            false
-        }
-    }
-
-    pub fn is_legal_action(&self, act: &Action) -> bool {
-        match act {
+        let state = maybe_state.unwrap();
+        let new_state_either = match &act {
             Action::Place(player, pos, stone) => match self.turn_order {
                 TurnOrder::BlackPlacesWhite => {
                     if (self.current_player == Player::Black)
                         && (*player == Player::White)
                         && (*stone == Stone::Flat)
                     {
-                        self.is_legal_place(player, pos, stone)
+                        state.place_stone(*player, pos, *stone)
                     } else {
-                        (self.log)(String::from(
+                        Err(String::from(
                             "At the start of the game, B must place a W flat.",
-                        ));
-                        false
+                        ))
                     }
                 }
                 TurnOrder::WhitePlacesBlack => {
@@ -518,64 +500,53 @@ impl<L: Fn(String)> Game<L> {
                         && (*player == Player::Black)
                         && (*stone == Stone::Flat)
                     {
-                        self.is_legal_place(player, pos, stone)
+                        state.place_stone(*player, pos, *stone)
                     } else {
-                        (self.log)(String::from(
+                        Err(String::from(
                             "At the start of the game, W must place a B flat.",
-                        ));
-                        false
+                        ))
                     }
                 }
-                TurnOrder::Normal => self.is_legal_place(player, pos, stone),
+                TurnOrder::Normal => state.place_stone(*player, pos, *stone),
             },
             Action::Move(player, pos, direction, drops) => match self.turn_order {
                 TurnOrder::Normal => {
                     if *player == self.current_player {
-                        self.is_legal_move(player, pos, direction, drops)
+                        state.move_stack(*player, pos, *direction, drops)
                     } else {
-                        (self.log)(format!(
+                        Err(format!(
                             "{} may not take actions on {}'s turn.",
                             player, self.current_player
-                        ));
-                        false
+                        ))
                     }
                 }
-                _ => {
-                    (self.log)(String::from(
-                        "At the start of the game only placing flats is allowed.",
-                    ));
-                    false
-                }
+                _ => Err(String::from(
+                    "At the start of the game only placing flats is allowed.",
+                )),
             },
-        }
-    }
+        };
 
-    pub fn perform_action(&mut self, act: Action) -> bool {
-        if self.is_legal_action(&act) {
-            let state = &self.states[self.states.len() - 1];
-            let new_state = match &act {
-                Action::Place(player, pos, stone) => state.place_stone(*player, pos, *stone),
-                Action::Move(_, pos, direction, drops) => state.move_stack(pos, *direction, drops),
-            };
-            self.states.push(new_state);
-            self.actions.push(act);
-            self.current_player = match self.current_player {
-                Player::Black => Player::White,
-                Player::White => Player::Black,
-            };
-            self.turn_order = match self.turn_order {
-                TurnOrder::BlackPlacesWhite => TurnOrder::WhitePlacesBlack,
-                TurnOrder::WhitePlacesBlack => TurnOrder::Normal,
-                TurnOrder::Normal => TurnOrder::Normal,
-            };
-            true
-        } else {
-            false
+        match new_state_either {
+            Err(e) => return Err(e),
+            Ok(new_state) => {
+                self.states.push(new_state);
+                self.actions.push(act);
+                self.current_player = match self.current_player {
+                    Player::Black => Player::White,
+                    Player::White => Player::Black,
+                };
+                self.turn_order = match self.turn_order {
+                    TurnOrder::BlackPlacesWhite => TurnOrder::WhitePlacesBlack,
+                    TurnOrder::WhitePlacesBlack => TurnOrder::Normal,
+                    TurnOrder::Normal => TurnOrder::Normal,
+                };
+                return Ok(());
+            }
         }
     }
 }
 
-impl<L: Fn(String)> fmt::Display for Game<L> {
+impl fmt::Display for Game {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
         write!(
             f,