6
\$\begingroup\$

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 to the official Rust Programming Language Forum: Rust Novice’s Tic Tac Toe)

\$\endgroup\$
1
\$\begingroup\$

You've already done a good job cleaning things up, and running clippy puts you far ahead of other code I've seen. I'll be writing recommendations for other stuff you could add to make it nicer for the user, but is in no way necessary.

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

Just mentioning that maplit has a macro to do this all in one. As I said earlier, there's no reason to do so, but it's an option.

 

pub fn run(&mut self) {}
fn display_help(&self) {}

I'd refactor this to define the commands and their help messages in one, and then just iterate over them to print help and match input. You could do a .map(str::len).max() to get the largest command for right aligning. I'd also get rid of the extra space in between each entry.

However, if you want an even cooler selection menu, I'd check out dialoguer. It will allow you to select what you want with up and down arrows to select an option: Selection prompt

 

fn rows(&self) -> impl Iterator<Item = impl Iterator<Item = &Cell>> {}
fn columns(&self) -> impl Iterator<Item = impl Iterator<Item = &Cell>> {}
fn diagonals(&self) -> impl Iterator<Item = impl Iterator<Item = &Cell>> {}

Those should return Iterators over Cells, not &Cells. Just throw a .copied() at the end. You can then remove occupied_by's lifetime stuff.

 

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))
}

Instead of asking if someone won, ask who won. See if the row is all_equal, and if so, return that Player. You'll probably be using Option::or_else for that.

 

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

Error types should implement Error.

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?

Yes, it allows people to use it in many different use cases. Someone could take it and then embed it in their own games program that bundles a bunch of games together.

Am I over-engineering everything?

Yeah you are. Tic-tac-toe, by definition, has two players. And you enforce that through the use of the Player enum. Then, you go and use a HashMap to store scores. There's no need to do this. Just store the X and O scores as two integers.

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?

Yes! Do things like that whenever you can.

 

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
            ))
        })
    })

Replace .or_else(|_| Err("Invalid move".to_owned())) with .map_err(|_| "Invalid move".to_owned()). Even better, use anyhow's .with_context(), because Strings aren't the best error type as they don't implement Error.

Here's that part rewritten with anyhow:

use anyhow::{bail, Context};
if let Err(message) = input
    .parse()
    .context("Invalid move")
    .and_then(|pos| Pos::new(pos).context("Invalid position"))
    .and_then(|pos| self.board.place(pos, player).map_err(Into::into))
{
    eprintln!("{:#}", message);
    continue;
}

That depends on implementing Error for PlaceError:

impl fmt::Display for PlaceError {
    fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
        write!(f, "Position {} occupied by {}", self.pos, self.occupied_by)
    }
}
impl std::error::Error for PlaceError {}
\$\endgroup\$
0
\$\begingroup\$

In addition to lights0123's excellent answer:

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

can be simplified with matches!:

fn is_vacant(self) -> bool {
    matches!(self, Cell::Vacant)
}

or, with Eq,

fn is_vacant(self) -> bool {
    self == Cell::Vacant
}

is_vacant and is_occupied also probably make more sense as pub functions.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.