1
\$\begingroup\$

I implemented a simple CLI based rock-paper-scissors game in Rust. The player plays against the computer. Each game has three rounds, not counting draws.

Cargo.toml

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

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

[dependencies]
rand = "0.8.5"

lib.rs

use rand::{
    distributions::{Distribution, Standard},
    Rng,
};
use std::cmp::Ordering;
use std::fmt::{Debug, Display, Formatter};
use std::str::FromStr;

#[derive(Clone, Copy, Debug, Eq, PartialEq)]
pub enum RPS {
    Rock,
    Paper,
    Scissors,
}

impl Display for RPS {
    fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result {
        write!(
            f,
            "{}",
            match self {
                Self::Rock => '🪨',
                Self::Paper => '🧻',
                Self::Scissors => '✀',
            }
        )
    }
}

impl PartialOrd<Self> for RPS {
    fn partial_cmp(&self, other: &Self) -> Option<Ordering> {
        match (self, other) {
            (Self::Rock, Self::Paper) => Some(Ordering::Less),
            (Self::Rock, Self::Scissors) => Some(Ordering::Greater),
            (Self::Paper, Self::Rock) => Some(Ordering::Greater),
            (Self::Paper, Self::Scissors) => Some(Ordering::Less),
            (Self::Scissors, Self::Rock) => Some(Ordering::Less),
            (Self::Scissors, Self::Paper) => Some(Ordering::Greater),
            (_, _) => None,
        }
    }
}

impl Ord for RPS {
    fn cmp(&self, other: &Self) -> Ordering {
        match self.partial_cmp(other) {
            Some(order) => order,
            None => Ordering::Equal,
        }
    }
}

impl Distribution<RPS> for Standard {
    fn sample<R: Rng + ?Sized>(&self, rng: &mut R) -> RPS {
        match rng.gen_range(0..=2) {
            0 => RPS::Rock,
            1 => RPS::Paper,
            _ => RPS::Scissors,
        }
    }
}

impl FromStr for RPS {
    type Err = &'static str;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        match s.trim().to_lowercase().as_str() {
            "r" => Ok(Self::Rock),
            "p" => Ok(Self::Paper),
            "s" => Ok(Self::Scissors),
            _ => Err("invalid string"),
        }
    }
}

main.rs

use rand::random;
use rps::RPS;
use std::cmp::Ordering;
use std::io::{stdin, stdout, Write};
use std::str::FromStr;

fn main() {
    let mut rounds: u8 = 0;
    let mut score: i8 = 0;

    loop {
        if rounds >= 3 {
            break;
        }

        let computer: RPS = random();
        let user = read_user();

        match user.cmp(&computer) {
            Ordering::Equal => {
                println!("Draw: {} = {}", user, computer);
                continue;
            }
            Ordering::Less => {
                println!("You lost: {} < {}", user, computer);
                score -= 1;
            }
            Ordering::Greater => {
                println!("You won: {} > {}", user, computer);
                score += 1;
            }
        }

        rounds += 1;
    }

    match score.cmp(&0) {
        Ordering::Equal => println!("The game ended in a draw."),
        Ordering::Less => println!("The computer won the game."),
        Ordering::Greater => println!("The player won the game."),
    }
}

fn read_user() -> RPS {
    loop {
        print!("Your selection (r/p/s): ");
        stdout().flush().expect("Could not flush STDOUT.");
        let mut buf = String::new();

        if stdin().read_line(&mut buf).is_ok() {
            match RPS::from_str(&buf) {
                Ok(rps) => return rps,
                Err(error) => println!("{}: {}", error, buf),
            }
        }
    }
}

I'd like to have feedback on the general code style and especially the use of trait implementations in lib.rs.

\$\endgroup\$

1 Answer 1

1
\$\begingroup\$

PartialOrd and None

You're not supposed to return None in partial_cmp() when a and b are equal. You're supposed to return Some(Ordering::Equal). Only weird types that have no total ordering return None, such as NaN and Nan in floats, where it is neither == nor !=.

If you do have total ordering, it is better to implement the logic in Ord to make sure you never return None, and then implement PartialOrd in terms of it: Some(self.cmp(other)).

PartialOrd<Self>

The <Self> is redundant. Self is the default.

Stdout and Stdin

I wouldn't do this in this program since the I/O is definitely not a performance problem, but if you're doing a lot of I/O know that you can lock stdin and stdout for faster access to them (but this means you cannot access them without the lock), and you can also wrap them with buffering to make them even faster:

use std::io::{stdin, stdout, BufRead, BufReader, BufWriter, Write};

fn read_user() -> RPS {
    let mut stdout = BufWriter::new(stdout().lock());
    let mut stdin = BufReader::new(stdin().lock());
    loop {
        write!(stdout, "Your selection (r/p/s): ").unwrap();
        stdout.flush().expect("Could not flush STDOUT.");
        let mut buf = String::new();

        if stdin.read_line(&mut buf).is_ok() {
            match RPS::from_str(&buf) {
                Ok(rps) => return rps,
                Err(error) => println!("{}: {}", error, buf),
            }
        }
    }
}

Using ThreadRng explicitly

Again, this doesn't matter in this example because the code is not performance sensitive, but it is faster to store the ThreadRng in a variable instead of calling random() if you do it many times:

use rand::Rng;

fn main() {
    let mut rounds: u8 = 0;
    let mut score: i8 = 0;

    let mut rng = rand::thread_rng();
    
    loop {
        if rounds >= 3 {
            break;
        }

        let computer: RPS = rng.gen();
    // ...
}

use std::fmt::Debug is redundant.

Because Debug is already in the prelude. Not the trait - the derive macro. The compiler doesn't warn about that unfortunately (but see clippy#1124).

Don't use Ord or PartialOrd

By the contract of PartialOrd (and Ord), it should be transitive (a < b and b < c means a < c). Your implementation does not fulfill that criteria (e.g. Rock < Paper and Paper < Scissors but Rock > Scissors), therefore you cannot implement PartialOrd.

Use your own method to compare RPS and checks who win. I recommend also using your own enum for the result (and not Ordering), because for me at least, Ordering::Greater for example does not clearly mean left wins right.

\$\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.