6
\$\begingroup\$

Intro

In order to get familiar with and improve my Rust skills I coded Tic-Tac-Toe. I want to write good Rust code, any feedback regarding why my code isn't in a Rust style or improvements that could be made would be very helpful.

Question

Is there a better way to implement my "new" method for the Ttt struct? I don't particularly like manually passing the array ['0', '1', '2', '3', '4', '5', '6', '7', '8'] to both board and options: Ttt::create_options(), but I haven't seen a way around doing it like that.

Code

lib.rs

pub mod tictactoe {
    use std::collections::HashMap;
    use std::io;

    const BOARD_SIZE: usize = 9;
    const P1: char = 'X';
    const P2: char = 'O';
    pub struct Ttt {
        pub board: [char; BOARD_SIZE],
        pub options: HashMap<usize, char>,
        pub active_player: char,
        pub game_over: bool,
        pub winner: char,
    }

    impl Ttt {
        fn create_options(board: [char; BOARD_SIZE]) -> HashMap<usize, char> {
            let mut options = HashMap::new();
            for space in 0..board.len() {
                options.insert(space, board[space]);
            }

            return options;
        }

        // I feel like this needs improvement
        pub fn new() -> Ttt {
            Ttt {
                board: ['0', '1', '2', '3', '4', '5', '6', '7', '8'],
                options: Ttt::create_options(['0', '1', '2', '3', '4', '5', '6', '7', '8']),
                active_player: P1,
                game_over: false,
                winner: ' ',
            }
        }

        pub fn play(&mut self) -> char {
            while !self.game_over {
                self.print_board();

                let pick = self.get_input();
                self.options.remove(&pick);
                self.board[pick] = self.active_player;

                self.game_over = self.is_game_over();

                if self.game_over {
                    self.winner = self.active_player;
                } else {
                    self.switch_player();
                }
            }

            self.print_board();
            println!("\n{} is the winner!", self.winner);
            return self.winner;
        }

        pub fn is_game_over(&self) -> bool {
            if self.options.len() < 1 {
                println!("It's a tie!");
                return true;
            }
            if self.board[0] == self.board[1] && self.board[0] == self.board[2] {
                println!("Top row win!");
                return true;
            }
            if self.board[3] == self.board[4] && self.board[3] == self.board[5] {
                println!("Middle row win!");
                return true;
            }
            if self.board[6] == self.board[7] && self.board[6] == self.board[8] {
                println!("Bottom row win!");
                return true;
            }
            if self.board[0] == self.board[3] && self.board[0] == self.board[6] {
                println!("Left column win!");
                return true;
            }
            if self.board[1] == self.board[4] && self.board[1] == self.board[7] {
                println!("Center column win!");
                return true;
            }
            if self.board[2] == self.board[5] && self.board[2] == self.board[8] {
                println!("Right column win!");
                return true;
            }
            if self.board[0] == self.board[4] && self.board[0] == self.board[8] {
                println!("Backslash win!");
                return true;
            }
            if self.board[2] == self.board[4] && self.board[2] == self.board[6] {
                println!("Forwardslash win!");
                return true;
            }

            return false;
        }

        pub fn switch_player(&mut self) {
            if self.active_player == P1 {
                self.active_player = P2;
            } else {
                self.active_player = P1;
            }
        }

        pub fn print_board(&self) {
            println!(
                " {} | {} | {} ",
                self.board[0], self.board[1], self.board[2]
            );
            println!("-----------");
            println!(
                " {} | {} | {} ",
                self.board[3], self.board[4], self.board[5]
            );
            println!("-----------");
            println!(
                " {} | {} | {} ",
                self.board[6], self.board[7], self.board[8]
            );
        }

        pub fn get_options(&self) -> &HashMap<usize, char> {
            return &self.options;
        }

        pub fn get_input_ML(&self, choice: usize) -> usize {
            return choice;
        }

        pub fn get_input(&self) -> usize {
            println!("Choose a selection from the grid.");
            loop {
                let mut coord = String::new();
                io::stdin()
                    .read_line(&mut coord)
                    .expect("Failed to read line");
                let coord: usize = match coord.trim().parse() {
                    Ok(num) => num,
                    Err(_) => {
                        println!("Please enter a valid selection.");
                        continue;
                    }
                };
                if !self.options.contains_key(&coord) {
                    println!("Please enter a valid selection.");
                    continue;
                }
                return coord;
            }
        }
    }
}

main.rs

use ttt::tictactoe::Ttt;

fn main() {
    let mut g = Ttt::new();
    g.play();
}

Cargo.toml

[package]
name = "ttt"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]

Any feedback is much appreciated. Thank you.

\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

Just put it in a local value for brevity. That's it.

pub fn new() -> Ttt {
    let board = ['0', '1', '2', '3', '4', '5', '6', '7', '8'];
    Ttt {
        board,
        options: Ttt::create_options(board),
        active_player: P1,
        game_over: false,
        winner: ' ',
    }
}

Edit: and this looks fine to me, I'm not sure what kind of improvement you're looking for. I can't see any potential for clear improvement here. You may intialize options to an empty HashMap and update it after you construct the Ttt. That way, you wouldn't pass anything explicitly.

You may determine whether the game is a tie earlier, when there is one vacant square rather than with no remaining vacant squares. I leave that as a potential exercise for you.

I feel like you should abandon this implementation of options and use board as the only source of info about the board.

I would do vacant_coords.

fn vacant_coords(&self) -> impl Iterator<Item=usize> + '_ {
    self.board.iter().enumerate().filter_map(|(i, &ch)| {
        if ch == P1 || ch == P2 {
            None
        } else {
            Some(i)
        }
    })
}

Use that to check for number of vacant coords

if self.vacant_coords().count() == 0 {
    println!("It's a tie!");
    return true;
}

Use that to check whether input is a vacant coord

if !self.vacant_coords().find(|&i| i == coord).is_some() {
    println!("Please enter a valid selection.");
    continue;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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