Rust Novice's Tic Tac Toe

code review
1

Introduction

I am a Rust novice. So far, I have finished reading the first 15 chapters of The Rust Programming Language (a.k.a. the book). Here's my first big Rust project — Tic Tac Toe.

Each invocation of the program starts a session, in which the scores of two players O and X are tracked on an internal scoreboard. The program starts with a session menu, which supports a bunch of commands. For example, the scoreboard command displays the scores, and the start command starts a game (optionally specifying who is the first player). Once a game is started, the board is displayed, and the players are asked to enter their move. See the Example Session below for more information.

I have run rustfmt and clippy on my code, and improved my code according to their feedback. I would like to have a code review, since I want to become aware of my mistakes and avoid making them again. See the Specific Concerns below for more information.

Code

src/board.rs

use std::fmt;
use std::hash::Hash;
use std::iter;
use std::str;
use std::usize;

use itertools::Itertools;

#[derive(Clone, Copy, Debug, Eq, Hash, PartialEq)]
pub enum Player {
    Nought,
    Cross,
}

impl Player {
    pub fn toggle(self) -> Player {
        match self {
            Player::Nought => Player::Cross,
            Player::Cross => Player::Nought,
        }
    }
}

impl fmt::Display for Player {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            Player::Nought => write!(f, "O"),
            Player::Cross => write!(f, "X"),
        }
    }
}

impl str::FromStr for Player {
    type Err = ParsePlayerError;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s {
            "O" => Ok(Player::Nought),
            "X" => Ok(Player::Cross),
            _ => Err(ParsePlayerError {}),
        }
    }
}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct ParsePlayerError {}

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum Cell {
    Occupied(Player),
    Vacant,
}

impl Cell {
    fn is_occupied(self) -> bool {
        !self.is_vacant()
    }

    fn is_vacant(self) -> bool {
        match self {
            Cell::Occupied(_) => false,
            Cell::Vacant => true,
        }
    }
}

impl fmt::Display for Cell {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        match self {
            Cell::Occupied(player) => write!(f, "{}", player),
            Cell::Vacant => write!(f, " "),
        }
    }
}

// a position on the board
// 1 2 3
// 4 5 6
// 7 8 9
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub struct Pos {
    pos: usize,
}

impl Pos {
    pub fn new(pos: usize) -> Option<Pos> {
        if (1..=Board::SIZE).contains(&pos) {
            Some(Pos { pos })
        } else {
            None
        }
    }
    pub fn get(self) -> usize {
        self.pos
    }
}

impl fmt::Display for Pos {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        write!(f, "{}", self.get())
    }
}

pub struct Board {
    // row-major layer
    cells: [Cell; Board::SIZE],
}

impl Board {
    pub const WIDTH: usize = 3;
    pub const SIZE: usize = Board::WIDTH * Board::WIDTH;

    pub fn new() -> Board {
        Board {
            cells: [Cell::Vacant; Board::SIZE],
        }
    }

    pub fn place(&mut self, pos: Pos, player: Player) -> Result<(), PlaceError> {
        let cell = &mut self.cells[pos.get() - 1];
        match *cell {
            Cell::Occupied(player) => Err(PlaceError {
                pos,
                occupied_by: player,
            }),
            Cell::Vacant => {
                *cell = Cell::Occupied(player);
                Ok(())
            }
        }
    }

    pub fn wins(&self, player: Player) -> bool {
        self.rows().any(|row| occupied_by(row, player))
            || self.columns().any(|column| occupied_by(column, player))
            || self
                .diagonals()
                .any(|diagonal| occupied_by(diagonal, player))
    }

    pub fn is_draw(&self) -> bool {
        self.is_complete() && !self.wins(Player::Nought) && !self.wins(Player::Cross)
    }

    fn rows(&self) -> impl Iterator<Item = impl Iterator<Item = &Cell>> {
        self.cells.chunks(Board::WIDTH).map(|chunk| chunk.iter())
    }

    fn columns(&self) -> impl Iterator<Item = impl Iterator<Item = &Cell>> {
        (0..Board::WIDTH).map(move |n| self.cells.iter().skip(n).step_by(Board::WIDTH))
    }

    fn diagonals(&self) -> impl Iterator<Item = impl Iterator<Item = &Cell>> {
        // major and minor have the same type
        let major = iter::once(
            self.cells
                .iter()
                .skip(0)
                .step_by(Board::WIDTH + 1)
                .take(Board::WIDTH),
        );
        let minor = iter::once(
            self.cells
                .iter()
                .skip(Board::WIDTH - 1)
                .step_by(Board::WIDTH - 1)
                .take(Board::WIDTH),
        );
        major.chain(minor)
    }

    fn is_complete(&self) -> bool {
        self.cells.iter().all(|cell| cell.is_occupied())
    }
}

impl fmt::Display for Board {
    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
        writeln!(f, "+{}+", ["---"; Board::WIDTH].join("+"))?;

        for row in self.rows() {
            writeln!(f, "| {} |", row.format(" | "))?;
            writeln!(f, "+{}+", ["---"; Board::WIDTH].join("+"))?;
        }

        Ok(())
    }
}

fn occupied_by<'a, I: Iterator<Item = &'a Cell>>(mut cells: I, player: Player) -> bool {
    cells.all(|cell| *cell == Cell::Occupied(player))
}

#[derive(Debug, Eq, PartialEq)]
pub struct PlaceError {
    pub pos: Pos,
    pub occupied_by: Player,
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn player_toggle() {
        assert_eq!(Player::Nought, Player::Cross.toggle());
        assert_eq!(Player::Cross, Player::Nought.toggle());
    }

    #[test]
    fn player_display() {
        assert_eq!("O", format!("{}", Player::Nought));
        assert_eq!("X", format!("{}", Player::Cross));
    }

    #[test]
    fn player_parse() {
        assert_eq!(Ok(Player::Nought), "O".parse());
        assert_eq!(Ok(Player::Cross), "X".parse());

        assert!("".parse::<Player>().is_err());
        assert!("a".parse::<Player>().is_err());
        assert!("o".parse::<Player>().is_err());
        assert!("XXX".parse::<Player>().is_err());
    }

    #[test]
    fn cell() {
        assert!(Cell::Occupied(Player::Nought).is_occupied());
        assert!(Cell::Occupied(Player::Cross).is_occupied());
        assert!(!Cell::Vacant.is_occupied());

        assert!(!Cell::Occupied(Player::Nought).is_vacant());
        assert!(!Cell::Occupied(Player::Cross).is_vacant());
        assert!(Cell::Vacant.is_vacant());
    }

    #[test]
    fn cell_display() {
        assert_eq!("O", format!("{}", Cell::Occupied(Player::Nought)));
        assert_eq!("X", format!("{}", Cell::Occupied(Player::Cross)));
        assert_eq!(" ", format!("{}", Cell::Vacant));
    }

    #[test]
    fn pos() {
        assert_eq!(1, Pos::new(1).unwrap().get());
        assert_eq!(4, Pos::new(4).unwrap().get());
        assert_eq!(9, Pos::new(9).unwrap().get());

        assert!(Pos::new(0).is_none());
        assert!(Pos::new(10).is_none());
        assert!(Pos::new(usize::MAX).is_none());
    }

    #[test]
    fn board_new() {
        let board = Board::new();
        assert_eq!([Cell::Vacant; 9], board.cells);
    }

    #[test]
    fn board_place() {
        let mut board = Board::new();

        board.place(Pos::new(1).unwrap(), Player::Nought).unwrap();
        assert_eq!(
            [
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
            ],
            board.cells
        );
        board.place(Pos::new(5).unwrap(), Player::Cross).unwrap();
        board.place(Pos::new(9).unwrap(), Player::Nought).unwrap();
        assert_eq!(
            [
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Vacant,
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
            ],
            board.cells
        );

        assert_eq!(
            PlaceError {
                pos: Pos::new(1).unwrap(),
                occupied_by: Player::Nought,
            },
            board
                .place(Pos::new(1).unwrap(), Player::Cross)
                .unwrap_err()
        );
    }

    #[test]
    fn board_display() {
        assert_eq!(
            "\
            +---+---+---+\n\
            |   |   |   |\n\
            +---+---+---+\n\
            |   |   |   |\n\
            +---+---+---+\n\
            |   |   |   |\n\
            +---+---+---+\n\
            ",
            format!("{}", Board::new()),
        );
    }

    #[test]
    fn board_rows() {
        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };

        let mut rows = board.rows();

        let mut row = rows.next().unwrap();
        assert_eq!(Cell::Occupied(Player::Nought), *row.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Cross), *row.next().unwrap());
        assert_eq!(Cell::Vacant, *row.next().unwrap());
        assert!(row.next().is_none());

        let mut row = rows.next().unwrap();
        assert_eq!(Cell::Occupied(Player::Cross), *row.next().unwrap());
        assert_eq!(Cell::Vacant, *row.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Nought), *row.next().unwrap());
        assert!(row.next().is_none());

        let mut row = rows.next().unwrap();
        assert_eq!(Cell::Vacant, *row.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Nought), *row.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Cross), *row.next().unwrap());
        assert!(row.next().is_none());

        assert!(rows.next().is_none());
    }

    #[test]
    fn board_columns() {
        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };

        let mut columns = board.columns();

        let mut column = columns.next().unwrap();
        assert_eq!(Cell::Occupied(Player::Nought), *column.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Cross), *column.next().unwrap());
        assert_eq!(Cell::Vacant, *column.next().unwrap());
        assert!(column.next().is_none());

        let mut column = columns.next().unwrap();
        assert_eq!(Cell::Occupied(Player::Cross), *column.next().unwrap());
        assert_eq!(Cell::Vacant, *column.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Nought), *column.next().unwrap());
        assert!(column.next().is_none());

        let mut column = columns.next().unwrap();
        assert_eq!(Cell::Vacant, *column.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Nought), *column.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Cross), *column.next().unwrap());
        assert!(column.next().is_none());

        assert!(columns.next().is_none());
    }

    #[test]
    fn board_diagonals() {
        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };

        let mut diagonals = board.diagonals();

        let mut diagonal = diagonals.next().unwrap();
        assert_eq!(Cell::Occupied(Player::Nought), *diagonal.next().unwrap());
        assert_eq!(Cell::Vacant, *diagonal.next().unwrap());
        assert_eq!(Cell::Occupied(Player::Cross), *diagonal.next().unwrap());
        assert!(diagonal.next().is_none());

        let mut diagonal = diagonals.next().unwrap();
        assert_eq!(Cell::Vacant, *diagonal.next().unwrap());
        assert_eq!(Cell::Vacant, *diagonal.next().unwrap());
        assert_eq!(Cell::Vacant, *diagonal.next().unwrap());
        assert!(diagonal.next().is_none());

        assert!(diagonals.next().is_none());
    }

    #[test]
    fn board_is_complete() {
        let board = Board {
            cells: [Cell::Occupied(Player::Cross); 9],
        };
        assert!(board.is_complete());

        let board = Board {
            cells: [Cell::Vacant; 9],
        };
        assert!(!board.is_complete());

        let board = Board {
            cells: [
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };
        assert!(!board.is_complete());
    }

    #[test]
    fn board_wins() {
        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };
        assert!(!board.wins(Player::Nought));
        assert!(!board.wins(Player::Cross));

        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Nought),
            ],
        };
        assert!(board.wins(Player::Nought));
        assert!(!board.wins(Player::Cross));
    }

    #[test]
    fn board_is_draw() {
        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Cross),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Vacant,
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };
        assert!(!board.is_draw());

        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Nought),
            ],
        };
        assert!(!board.is_draw());

        let board = Board {
            cells: [
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
                Cell::Occupied(Player::Nought),
                Cell::Occupied(Player::Cross),
            ],
        };
        eprintln!("{}", board);
        assert!(board.is_draw());
    }
}

src/game.rs

use std::io;

use crate::board::{Board, Player, Pos};
use crate::utility;

pub enum Result {
    Win(Player),
    Draw,
}

pub struct Game {
    board: Board,
    first_player: Player,
    resigned: Option<Player>,
}

impl Game {
    pub fn new(first_player: Player) -> Game {
        Game {
            board: Board::new(),
            first_player,
            resigned: Option::None,
        }
    }

    pub fn run(&mut self) -> Result {
        let mut current_player = self.first_player;

        loop {
            self.process_move(current_player);

            if let Some(player) = self.resigned {
                utility::clear_screen();
                print!("{}", self.board);

                let winner = player.toggle();
                println!("{} wins by resignation.", winner);
                return Result::Win(winner);
            } else if self.board.wins(current_player) {
                utility::clear_screen();
                print!("{}", self.board);
                println!("{} wins.", current_player);
                return Result::Win(current_player);
            } else if self.board.is_draw() {
                utility::clear_screen();
                print!("{}", self.board);
                println!("It's a draw.");
                return Result::Draw;
            }

            current_player = current_player.toggle()
        }
    }

    fn process_move(&mut self, player: Player) {
        loop {
            utility::clear_screen();

            print!("{}", self.board);
            println!("[{}] Enter your move: ('help' for help)", player);

            let mut input = String::new();
            io::stdin()
                .read_line(&mut input)
                .expect("Failed to read input");

            let input = input.trim();
            match input {
                "help" => {
                    println!();
                    self.display_move_help(player);
                    continue;
                }
                "resign" => {
                    self.resigned = Some(player);
                    break;
                }
                _ => {}
            }

            if let Err(message) = input
                .parse()
                .or_else(|_| Err("Invalid move".to_owned()))
                .and_then(|pos| Pos::new(pos).ok_or_else(|| "Invalid position".to_owned()))
                .and_then(|pos| {
                    self.board.place(pos, player).or_else(|place_error| {
                        Err(format!(
                            "Position {} occupied by {}",
                            place_error.pos, place_error.occupied_by
                        ))
                    })
                })
            {
                eprintln!("{}", message);
                continue;
            }

            break;
        }
    }

    fn display_move_help(&self, player: Player) {
        print!(
            "\
            Supported commands:                         \n\
                                                        \n\
            -   help: display help screen               \n\
                                                        \n\
            - resign: resign the game                   \n\
                                                        \n\
            -    1-9: place {} on the specified position\n\
                                                        \n\
            *       +---+---+---+                       \n\
            *       | 1 | 2 | 3 |                       \n\
            *       +---+---+---+                       \n\
            *       | 4 | 5 | 6 |                       \n\
            *       +---+---+---+                       \n\
            *       | 7 | 8 | 9 |                       \n\
            *       +---+---+---+                       \n\
            ",
            player
        );
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn game_new() {
        let game = Game::new(Player::Nought);

        assert_eq!(
            "\
            +---+---+---+\n\
            |   |   |   |\n\
            +---+---+---+\n\
            |   |   |   |\n\
            +---+---+---+\n\
            |   |   |   |\n\
            +---+---+---+\n\
            ",
            format!("{}", game.board)
        );
        assert_eq!(Player::Nought, game.first_player);
        assert!(game.resigned.is_none());
    }
}

src/session.rs

use std::collections::HashMap;
use std::io;

use crate::board::Player;
use crate::game::{Game, Result};
use crate::utility;

pub struct Session {
    scores: HashMap<Player, u32>,
    first_player: Player,
}

impl Session {
    const DEFAULT_FIRST_PLAYER: Player = Player::Cross;

    pub fn new() -> Session {
        Session {
            scores: [(Player::Nought, 0), (Player::Cross, 0)]
                .iter()
                .copied()
                .collect(),
            first_player: Session::DEFAULT_FIRST_PLAYER,
        }
    }

    pub fn run(&mut self) {
        loop {
            utility::clear_screen();
            println!("Enter command: ('help' for help)");

            let mut input = String::new();
            io::stdin()
                .read_line(&mut input)
                .expect("Failed to read input");

            match input.trim() {
                "exit" | "quit" => break,
                "help" => {
                    println!();
                    self.display_help();
                }
                "reset" => self.reset_scores(),
                "scoreboard" => {
                    println!();
                    self.display_scoreboard();
                }
                input if input.starts_with("start") => {
                    self.process_start(input);
                }
                _ => {
                    eprintln!("Invalid command.");
                }
            }
        }
    }

    fn display_help(&self) {
        print!(
            "\
            Supported commands:                                                \n\
                                                                               \n\
            -       exit: quit the session                                     \n\
                                                                               \n\
            -       help: display help screen                                  \n\
                                                                               \n\
            -       quit: quit the session                                     \n\
                                                                               \n\
            -      reset: reset scores                                         \n\
                                                                               \n\
            - scoreboard: display scores                                       \n\
                                                                               \n\
            -      start: start a new game                                     \n\
                                                                               \n\
            -  start O/X: start a new game, with the specified first player    \n\
            "
        );
    }

    fn display_scoreboard(&self) {
        println!("Scoreboard:");

        let mut entries: Vec<_> = self.scores.iter().collect();
        entries.sort_unstable_by(|&(_, score_a), &(_, score_b)| score_b.cmp(score_a));
        for (player, score) in entries {
            println!();
            println!("- {}: {}", player, score);
        }
    }

    fn reset_scores(&mut self) {
        for score in self.scores.values_mut() {
            *score = 0;
        }
    }

    fn process_result(&mut self, result: Result) {
        match result {
            Result::Win(player) => *self.scores.get_mut(&player).unwrap() += 1,
            Result::Draw => {}
        }
    }

    fn process_start(&mut self, input: &str) {
        let args: Vec<_> = input.split_whitespace().collect();
        if !args.starts_with(&["start"]) || args.len() > 2 {
            eprintln!("Invalid command.");
            return;
        }

        if args.len() == 2 {
            self.first_player = match args[1].parse() {
                Ok(player) => player,
                Err(_) => {
                    eprintln!("Invalid player.");
                    return;
                }
            }
        }

        let mut game = Game::new(self.first_player);
        self.process_result(game.run());
        self.first_player = self.first_player.toggle();
    }
}

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn session_new() {
        let session = Session::new();

        assert_eq!(2, session.scores.len());
        assert_eq!(Some(&0), session.scores.get(&Player::Nought));
        assert_eq!(Some(&0), session.scores.get(&Player::Cross));
    }
}

src/utility.rs

pub fn clear_screen() {
    print!("\n\n");
}

src/lib.rs

mod board;
mod game;
mod session;
mod utility;

use session::Session;

pub fn run() {
    let mut session = Session::new();
    session.run();
}

src/main.rs

fn main() {
    tic_tac_toe::run();
}

Cargo.toml

[package]
name = "tic-tac-toe"
version = "0.1.0"
authors = ["L. F."]
edition = "2018"

[dependencies]
itertools = "0.9.0"

Example Session

Enter command: ('help' for help)
help

Supported commands:

-       exit: quit the session

-       help: display help screen

-       quit: quit the session

-      reset: reset scores

- scoreboard: display scores

-      start: start a new game

-  start O/X: start a new game, with the specified first player


Enter command: ('help' for help)
scoreboard

Scoreboard:

- O: 0

- X: 0


Enter command: ('help' for help)
start X


+---+---+---+
|   |   |   |
+---+---+---+
|   |   |   |
+---+---+---+
|   |   |   |
+---+---+---+
[X] Enter your move: ('help' for help)
5


+---+---+---+
|   |   |   |
+---+---+---+
|   | X |   |
+---+---+---+
|   |   |   |
+---+---+---+
[O] Enter your move: ('help' for help)
2


+---+---+---+
|   | O |   |
+---+---+---+
|   | X |   |
+---+---+---+
|   |   |   |
+---+---+---+
[X] Enter your move: ('help' for help)
4


+---+---+---+
|   | O |   |
+---+---+---+
| X | X |   |
+---+---+---+
|   |   |   |
+---+---+---+
[O] Enter your move: ('help' for help)
6


+---+---+---+
|   | O |   |
+---+---+---+
| X | X | O |
+---+---+---+
|   |   |   |
+---+---+---+
[X] Enter your move: ('help' for help)
7


+---+---+---+
|   | O |   |
+---+---+---+
| X | X | O |
+---+---+---+
| X |   |   |
+---+---+---+
[O] Enter your move: ('help' for help)
resign


+---+---+---+
|   | O |   |
+---+---+---+
| X | X | O |
+---+---+---+
| X |   |   |
+---+---+---+
X wins by resignation.


Enter command: ('help' for help)
scoreboard

Scoreboard:

- X: 1

- O: 0


Enter command: ('help' for help)
start


+---+---+---+
|   |   |   |
+---+---+---+
|   |   |   |
+---+---+---+
|   |   |   |
+---+---+---+
[O] Enter your move: ('help' for help)
2


+---+---+---+
|   | O |   |
+---+---+---+
|   |   |   |
+---+---+---+
|   |   |   |
+---+---+---+
[X] Enter your move: ('help' for help)
5


+---+---+---+
|   | O |   |
+---+---+---+
|   | X |   |
+---+---+---+
|   |   |   |
+---+---+---+
[O] Enter your move: ('help' for help)
4


+---+---+---+
|   | O |   |
+---+---+---+
| O | X |   |
+---+---+---+
|   |   |   |
+---+---+---+
[X] Enter your move: ('help' for help)
1


+---+---+---+
| X | O |   |
+---+---+---+
| O | X |   |
+---+---+---+
|   |   |   |
+---+---+---+
[O] Enter your move: ('help' for help)
9


+---+---+---+
| X | O |   |
+---+---+---+
| O | X |   |
+---+---+---+
|   |   | O |
+---+---+---+
[X] Enter your move: ('help' for help)
6


+---+---+---+
| X | O |   |
+---+---+---+
| O | X | X |
+---+---+---+
|   |   | O |
+---+---+---+
[O] Enter your move: ('help' for help)
8


+---+---+---+
| X | O |   |
+---+---+---+
| O | X | X |
+---+---+---+
|   | O | O |
+---+---+---+
[X] Enter your move: ('help' for help)
7


+---+---+---+
| X | O |   |
+---+---+---+
| O | X | X |
+---+---+---+
| X | O | O |
+---+---+---+
[O] Enter your move: ('help' for help)
3


+---+---+---+
| X | O | O |
+---+---+---+
| O | X | X |
+---+---+---+
| X | O | O |
+---+---+---+
It's a draw.


Enter command: ('help' for help)
scoreboard

Scoreboard:

- X: 1

- O: 0


Enter command: ('help' for help)
quit

Specific Concerns

  • I organized my code according to the Refactoring to Improve Modularity and Error Handling section of the book, but src/lib.rs and src/main.rs feel vacuous. Is this considered good design?

  • Compared to other implementation of Tic Tac Toe, my implementation seems extremely complicated. Am I over-engineering everything? Do I need to adhere more to the KISS principle?

  • I used impl Iterator<Item = impl Iterator<Item = &Cell>> as the return type of Board::rows, Board::columns, and Board::diagonals, because their implementations use different kinds of iterators. Is it OK to unify the return types like this?

  • I used a bit of functional programming in game.rs, which I'm not very familiar with:

    if let Err(message) = input
    .parse()
    .or_else(|_| Err("Invalid move".to_owned()))
    .and_then(|pos| Pos::new(pos).ok_or_else(|| "Invalid position".to_owned()))
    .and_then(|pos| {
        self.board.place(pos, player).or_else(|place_error| {
            Err(format!(
                "Position {} occupied by {}",
                place_error.pos, place_error.occupied_by
            ))
        })
    })

    I took a lot of time to write this, and it seems elusive. Should I simplify it?

Suggestions on all aspects of the code will be highly appreciated!

(Cross-posted from Code Review Stack Exchange: beginner - Rust Novice's Tic Tac Toe - Code Review Stack Exchange)

2

In general, this looks pretty good IMO (and it's got unit tests, which puts you ahead of most novice code I see, and a lot of my own code :laughing:)

I would like to focus on one point in particular from your post though:

In my opinion: yes, but perhaps not the way you're using them at the moment.

In an application like this, I'd usually divide it up like so:

  • lib.rs (and submodules) contain the business logic of the application, exposing a Rust-y public API.
  • main.rs implements the command line interface on top of that API.

This has several upsides:

  • It forces you to think about your API from the perspective of a third-party using your library, which in my experience, tends to lead to better design.
  • It makes it easier to split your logic out into a standalone library later on, or to build a new UI on top of it.

In your example, the parsing/printing logic is mixed in with your game logic, which I think might make things a little harder to test/maintain/understand in the long run.

A concrete example - rather than having process_move loop and read stdin directly, could you have it take a Move enum and return an error if the placement is invalid? Then all of that logic around parsing arguments could be pulled up to the top level, and your game module could focus purely on simulating the game itself.

As you say, this is all probably overkill for a game of tic-tac-toe :sweat_smile: But I think these are good principles to keep in mind for general Rust development. For example, ripgrep is a widely used command line tool written in Rust, but the CLI itself is just a wrapper around a library, so if you wanted to add grep functionality to another Rust tool, you could just pull that in.

4 Likes
3

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.