patternrustMinor

Rust cows and bulls

Submitted by: @import:stackexchange-codereview·Mar 10, 2026·

Viewed 0 times

rustandbullscows

Problem

I just wrote a cows and bulls implementation in Rust. It seems a little large. Are there any changes I could make while still keeping it easily readable?

```
extern crate rand;
use rand::Rng;
use std::{io,process,str};

fn input_check(s: &str, size: u32) -> bool{
//Check if number is positive valid number
let mut res = true;
let guess_num: u64 = match s.trim().parse() {
Ok(num) => num,
Err(_) => {
println!("Guess is not a valid ");
res = false;
return res;
}
};
//Check if the number contains unique numbers and it has the right number of characters
let unique_check: Vec = s.chars().collect();
let mut t = unique_check.clone();
t.dedup();
if t.len() != unique_check.len() {
println!("Guess must have unique characters ");
res = false;
return res;
}
if t.len() != size as usize {
println!("Guess is not of correct size ");
res = false;
return res;
}
res
}
fn main() {
//Main game loop
loop{
let mut secret_num_seed: Vec= (48..58).collect();
let mut rng = rand::thread_rng();
rng.shuffle(&mut secret_num_seed);
let mut in_size = String::new();
println!("Enter the number digit for the Secret number");
io::stdin().read_line(&mut in_size).expect("Failed to read");
let size: u32 = match in_size.trim().parse() {
Ok(num) => num,
Err(_) => {
println!("Enter a number please");
continue;
}
};
if size 10 {
println!("Enter a number between 1 and 10");
continue;
}
let mut secret_num = str::from_utf8(&mut secret_num_seed[0..size as usize]).unwrap();
println!("{:?}", secret_num);
println!("Secret number generated, let the games begin!");
let mut guess = String::new();
loop{
let (mut bulls, mut cows) = (0

Solution

-
Fix the compiler warnings; that's free advice you get just for compiling the code!

Unused variables should be removed or prefixed with a _ to indicate that they are unused on purpose.

Values assigned but never read are a waste of code and time; remove them.

Extra mutability is asking to shoot yourself in the foot later.

-
Use rustfmt. Notable issues in this code:

Use spaces after commas.

Use spaces before {

Use spaces after //

Use spaces around operators like =

Don't use spaces inside ().

else / else if on same line as }

-
res is a useless variable. Just return literals immediately from the function. This also allows you to remove more mutability.

-
Use if let for match statements where only one branch matters.

-
Prefer to let the compiler infer as much type information as it can. Collect into a Vec, for example, not a Vec.

-
Instead of commenting sections of a method, extract those sections to new methods, using the comment as the start of a name.

-
Vec::dedup is flat-out wrong. [1,2,1].dedupe() is [1,2,1]! Instead, use a data structure that naturally removes duplicates, like a HashSet.

-
Use Iterator::count to count an iterator instead of collecting it to a container.

-
Get the random number generator once before the loop, as has already been mentioned to you. Creating the RNG is a potentially expensive operation and it's better to avoid it.

-
Extract even more functions. Functions allow you to give names to sections of code and also provide natural lines to specify types.

-
It's wasteful to re-create and re-shuffle the array if the input is malformed. Check the input first to avoid that.

-
Creating a vector of numbers that are ASCII characters is very confusing. At the very least, create constants that would give names to the values - 58 is a magic number of the worst kind. Better than that, use byte literals (b'0') to express an ASCII character. Even better, write the code you want: an array of the characters 0 through 9. Then you can collect this into a String.

-
There's a useless &mut when calling input_check.

-
There's no need to declare and define two variables on the same line. Spread them out onto two lines.

-
There's no need to declare guess outside the loop - it doesn't need to persist between loop iterations. Moving it inside also removes the need to make it mutable.

-
Don't match on true / false, especially if you don't care about one branch. That's just an if

-
Declare variables as close to their usage site as they can be.

-
guess isn't trimmed in all the uses. You should trim it immediately and save that in a varaible to avoid that.

-
Don't use process:exit(0) just to exit the program. Just using return in the main method will exit with a success.

```
extern crate rand;

use rand::Rng;

use std::io;
use std::collections::HashSet;

fn is_positive_valid_number(s: &str) -> bool {
match s.trim().parse::() {
Ok(_) => true,
Err(_) => {
println!("Guess is not a valid ");
false
}
}
}

fn has_right_number_of_characters(s: &str, size: usize) -> bool {
if s.chars().count() != size {
println!("Guess is not of correct size ");
false
} else {
true
}
}

fn contains_unique_numbers(s: &str, size: usize) -> bool {
let unique_characters: HashSet = s.chars().collect();
if unique_characters.len() != size {
println!("Guess must have unique characters ");
false
} else {
true
}
}

fn input_check(s: &str, size: u32) -> bool {
let size = size as usize;

is_positive_valid_number(s) &&
has_right_number_of_characters(s, size) &&
contains_unique_numbers(s, size)
}

fn read_one_line() -> String {
let mut line = String::new();
io::stdin().read_line(&mut line).expect("Failed to read");
line
}

fn get_in_size() -> Result {
println!("Enter the number digit for the Secret number");

let in_size = read_one_line();

let size = match in_size.trim().parse() {
Ok(num) => num,
Err(_) => {
println!("Enter a number please");
return Err(());
}
};

if size 10 {
println!("Enter a number between 1 and 10");
return Err(());
}

Ok(size)
}

fn generate_secret_num(rng: &mut rand::ThreadRng, size: usize) -> String {
let mut secret_num_seed = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
rng.shuffle(&mut secret_num_seed);

secret_num_seed[0..size].iter().cloned().collect()
}

fn count_bulls_and_cows(guess: &str, secret_num: &str) -> (usize, usize) {
let mut bulls = 0;
let mut cows = 0;

for (i, j) in guess.chars().zip(secret_num.chars()) {
if i == j {
bulls += 1;
} else if secret_num.contains(i) {
cows += 1;
}
}

(bulls, cows)
}

fn main() {
let rng = &mut rand::thread_rng(

Code Snippets

extern crate rand;

use rand::Rng;

use std::io;
use std::collections::HashSet;

fn is_positive_valid_number(s: &str) -> bool {
    match s.trim().parse::<u64>() {
        Ok(_) => true,
        Err(_) => {
            println!("Guess is not a valid ");
            false
        }
    }
}

fn has_right_number_of_characters(s: &str, size: usize) -> bool {
    if s.chars().count() != size {
        println!("Guess is not of correct size ");
        false
    } else {
        true
    }
}

fn contains_unique_numbers(s: &str, size: usize) -> bool {
    let unique_characters: HashSet<_> = s.chars().collect();
    if unique_characters.len() != size {
        println!("Guess must have unique characters ");
        false
    } else {
        true
    }
}

fn input_check(s: &str, size: u32) -> bool {
    let size = size as usize;

    is_positive_valid_number(s) &&
        has_right_number_of_characters(s, size) &&
        contains_unique_numbers(s, size)
}


fn read_one_line() -> String {
    let mut line = String::new();
    io::stdin().read_line(&mut line).expect("Failed to read");
    line
}

fn get_in_size() -> Result<u32, ()> {
    println!("Enter the number digit for the Secret number");

    let in_size = read_one_line();

    let size = match in_size.trim().parse() {
        Ok(num) => num,
        Err(_) => {
            println!("Enter a number please");
            return Err(());
        }
    };

    if size < 1 || size > 10 {
        println!("Enter a number between 1 and 10");
        return Err(());
    }

    Ok(size)
}

fn generate_secret_num(rng: &mut rand::ThreadRng, size: usize) -> String {
    let mut secret_num_seed = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
    rng.shuffle(&mut secret_num_seed);

    secret_num_seed[0..size].iter().cloned().collect()
}

fn count_bulls_and_cows(guess: &str, secret_num: &str) -> (usize, usize) {
    let mut bulls = 0;
    let mut cows = 0;

    for (i, j) in guess.chars().zip(secret_num.chars()) {
        if i == j {
            bulls += 1;
        } else if secret_num.contains(i) {
            cows += 1;
        }
    }

    (bulls, cows)
}

fn main() {
    let rng = &mut rand::thread_rng();

    loop {
        let size = match get_in_size() {
            Ok(size) => size,
            Err(_) => continue,
        };

        let secret_num = generate_secret_num(rng, size as usize);
        println!("{:?}", secret_num);
        println!("Secret number generated, let the games begin!");

        loop {
            println!("Enter your guess");
            let guess = read_one_line();
            if !input_check(guess.trim(), size) {
                continue;
            }

            let (bulls, cows) = count_bulls_and_cows(&guess, &secret_num);

            if bulls == size as usize {
                println!("Congrats, you've won!, The number is: {} \nWould you like to play \
                          again Y?",
                         secret_num);
                let op_in = read_one_l

#[macro_use]
extern crate quick_error;
extern crate rand;

use rand::Rng;

use std::io;
use std::collections::HashSet;

quick_error!{
    #[derive(Debug)]
    enum Error {
        InvalidSize(err: std::num::ParseIntError) {
            description("Size is not a positive number")
            display("Size is not a positive number: {}", err)
        }
        SizeOutOfRange(size: u32) {
            description("Size must be between 1 and 10")
            display("Size must be between 1 and 10, but it is {}", size)
        }
        InvalidPositiveNumber(err: std::num::ParseIntError) {
            description("Guess is not a positive number")
            display("Guess is not a positive number: {}", err)
        }
        IncorrectSize(expected: usize, actual: usize) {
            description("Guess was not the correct length")
            display("Expected a guess with {} characters, but it was {}", expected, actual)
        }
        NotUnique {
            description("Guess did not contain unique characters")
        }
    }
}

type Result<T> = ::std::result::Result<T, Error>;

fn is_positive_valid_number(s: &str) -> Result<u32> {
    s.trim().parse().map_err(Error::InvalidPositiveNumber)
}

fn has_right_number_of_characters(s: &str, size: usize) -> Result<()> {
    let actual = s.chars().count();
    if actual != size {
        Err(Error::IncorrectSize(size, actual))
    } else {
        Ok(())
    }
}

fn contains_unique_numbers(s: &str, size: usize) -> Result<()> {
    let unique_characters: HashSet<_> = s.chars().collect();
    if unique_characters.len() != size {
        Err(Error::NotUnique)
    } else {
        Ok(())
    }
}

fn input_check(s: &str, size: u32) -> Result<()> {
    let size = size as usize;

    is_positive_valid_number(s)
        .and_then(|_| has_right_number_of_characters(s, size))
        .and_then(|_| contains_unique_numbers(s, size))
}


fn read_one_line() -> String {
    let mut line = String::new();
    io::stdin().read_line(&mut line).expect("Failed to read");
    line
}

fn get_in_size() -> Result<u32> {
    let in_size = read_one_line();

    let size = try!(in_size.trim().parse().map_err(Error::InvalidSize));

    if size < 1 || size > 10 {
        return Err(Error::SizeOutOfRange(size));
    }

    Ok(size)
}

fn generate_secret_num(rng: &mut rand::ThreadRng, size: usize) -> String {
    let mut secret_num_seed = ['0', '1', '2', '3', '4', '5', '6', '7', '8', '9'];
    rng.shuffle(&mut secret_num_seed);

    secret_num_seed[0..size].iter().cloned().collect()
}

fn count_bulls_and_cows(guess: &str, secret_num: &str) -> (usize, usize) {
    let mut bulls = 0;
    let mut cows = 0;

    for (i, j) in guess.chars().zip(secret_num.chars()) {
        if i == j {
            bulls += 1;
        } else if secret_num.contains(i) {
            cows += 1;
        }
    }

    (bulls, cows)
}

fn main() {
    let rng = &mut rand::thread_rng();

    loop {
        println!("Enter the number digit for the Secret number");

Context

StackExchange Code Review Q#134604, answer score: 6

Revisions (0)

No revisions yet.