Rust Guessing Game

Am I making good use of the Rust language and APIs? What can be improved?

extern crate rand;

use std::old_io;

fn main() {
    println!("Guessing Game");
    // declaring the type in the left side of let helps random() return the right datatype
    // An alternative is: let answer = (rand::random::() % 100) + 1;
    let answer: u8 = (rand::random() % 100) + 1;
    let mut guess: String;
    let mut guess_cast: Result;

    loop {
        guess = guesser();
        guess_cast = guess.trim().parse();

        let guess_num = match guess_cast {
            Ok(num) => num,
            Err(_) => { println!("Guess was not a number, try again"); continue; }
        };

        if guess_num  answer { println!("Too high") }
        else { println!("You guessed it!"); return; }
    }
}

fn guesser() -> String {
    println!("Guess a number between 0 and 100: ");
    old_io::stdin().read_line().ok().expect("Error getting user input")
}

Here’s more or less what I would write:

extern crate rand;

use std::io::{self, Write};

fn main() {
println!("Guessing Game");
let answer: u8 = (rand::random() % 100) + 1;

loop {
print!("Guess a number between 1 and 100: ");
let _ = io::stdout().flush();
let mut input = String::new();
io::stdin().read_line(&mut input).ok().expect("Error getting user input");

let guess = match input.trim().parse() {
Ok(num) => num,
Err(_) => {
println!("Guess was not a number, try again");
continue;
},
};

if guess answer {
println!("Too high");
} else {
println!("You guessed it!");
return;
}
}
}


I might replace that whole if block at the end with this instead (plus a use std::cmp::Ordering; and a : u8 on the let guess):

match guess.cmp(&answer) {
Ordering::Less => println!("Too low"),
Ordering::Greater => println!("Too high"),
Ordering::Equal => {
println!("You guessed it!");
return;
},
}


There are a number of changes, mostly subtle; some I will comment on, others I may omit:

• Make sure you get your bounds right (0 is not a possible number).

• Don’t use std::old_io, it’s on the way out.

• So long as you flush stdout explicitly, you can get the input on the same line as the query.

• Prefer semicolons at the end of blocks when producing () (by that, I mean cases like in the if block).

• With an if statement, either put the whole thing on one line or finish each line after the {.

• Avoid putting multiple statements on the same line.

• let mut a; loop { a = …; … } should only be preferred over loop { let a = …; … } if you’re going to use the value of a after the loop. Minimising the distance between definition and declaration and merging the two if possible is strongly preferable. As an aside, any place where you can drop a mut without convolution should also be taken.

• There doesn’t seem much point in splitting the acquisition of user input into a separate function.

• Using the same variable name with a variety of suffixes as you shift its format is OK, but if possible find nicer names. And when doing multiple steps, consider merging some lines together (guess_cast seems superfluous to me). I selected input for the user input and guess for the result, an arrangement that is both clearer and simpler in my opinion.

If you wanted to minimise allocations, you could shift the input definition out of the loop and replace its assignation inside the loop with input.clear(). That way it can keep using the same heap allocation time and time again. But that’s an optimisation that is not necessary for something like this. I might or might not do it, depending on what I had eaten, if anything, for morning tea.

Personally I think I’d go with break instead of return in this particular case, but that’s completely subjective.

StackExchange Code Review Q#84923, answer score: 6