unexplained Rust gotcha in Guessing Game example

[narration-sd]

This is due to a facet you are promoting of Rust language, but causes consternation in what is supposed to be an easy-entry example.

The problem occurs when you are invited to 'just' add a loop to your code-so-far, to allow multiple guesses. What you are likely to get if you follow other language practices is a nasty error:

thread 'main' panicked at 'Por favor, escriba un número!: ParseIntError { kind: InvalidDigit }', libcore\result.rs:1009:5
note: Run with `RUST_BACKTRACE=1` for a backtrace.
error: process didn't exit successfully: `target\debug\guessing_game.exe` (exit code: 101)

This occurs if you don't include the line let mut guess = String::new(); within the loop, which is likely if you have a lot of experience, and are in a hurry, especially, though no doubt newer persons could also do it. If you go on to include the better error handling later in the lesson, you'll get an equally obscure error: invalid digit found in string.

In the morning, it seems this occurs because of the type-shadowing which has made the var guess no longer able to take a String. I would also say that the error messages, either of them, are very uninformative about the real problem.

What can you do?

• I would suggest that the erroring for this kind of case be vastly improved. -- There's no line number for the code error -- this is basic, right? -- 'panicked' ?? come on... -- it's not an invalid digit, it's an invalid type String trying to be digits....you should say so -- this whole thing gets awkward fast, so examining how you treat 'shadowed' variables would likely lead to much better thinking about how to act when it goes wrong. -- So, this points to a fundamental Rust issue, I suspect. -- the RUST_BACKTRACE is actually an env var, thus unclear -- and its presence in the message is by reading your prior issues a sort of bodge to respond to other obscurities... -- the 'better' error message is just wrong. The input isn't bad because it's something wrong in the String; it's bad because the shadowed var can't take a String at all...

I was really surprised to see such a highly touted newer language being so raw. It makes me a bit wary about the intended reason to use it, Web Assembly. I hope you'll improve, thanks.

[steveklabnik]

Can you please re-file this against https://github.com/rust-lang/book? Thank you!

[steveklabnik]

Actually, I guess this really isn't about the contents of the book, it's more about the diagnostics. So let's leave it here. Sorry for the noise.

[narration-sd]

good man. And I edited a little to pick up own mis-types...

[estebank]

@steveklabnik I don't see how this is a diagnostics issue as it happens on runtime: someone writing this code from scratch would either have to deal with parse returning a Result::Err or know enough about what expect/unwrap is doing.

---

@narration-sd the book is using a blunt tool for error handling in order to keep the code simple. It is a very reasonable tradeoff. In real code you would deal with the result of

    let guess: Result<u32, ParseIntError> = guess.trim().parse();
    let guess: u32 = match guess {
        Ok(val) => val,
        Err(ParseIntError { kind: InvalidDigit}) => {
            println!("Por favor, escriba un número!");
            ::std::process::exit(1);
        }
        Err(err) => {
            println!("Se ha encontrado un error: {:?}", err);
            ::std::process::exit(1);
        }
    };

instead of failing outright when that parse has failed, as in the book:

    let guess: u32 = guess.trim().parse().expect("Please type a number!");

There's no line number for the code error -- this is basic, right?

This is because it isn't a compile time error, and there are reasons for panics to not carry more information than they already do.

'panicked' ?? come on...

It is the name of the unwinding mechanism in rust, the result of calling panic!() or expect on a Result.

it's not an invalid digit, it's an invalid type String trying to be digits....you should say so

The error is pointing that the valid String has invalid digits when trying to parse it as an u32.

the RUST_BACKTRACE is actually an env var, thus unclear

Perfectly fair.

the 'better' error message is just wrong. The input isn't bad because it's something wrong in the String; it's bad because the shadowed var can't take a String at all...

I believe you might be indeed misunderstanding where the problem is. It is not a type level problem, the string is still a valid string, the error is being caused when trying to call parse on a String that holds something that cannot be converted to a u32. If you remove the expect() call you will see that the result of parse can either be Ok(u32_val) or Err(IntParseErr). The later is the one being represented as the string invalid digit found in string.

---

I believe there are two actionable points are:

• Make it clear on panic that RUST_BACKTRACE is an env variable, perhaps even showing how to set it with the previous unsuccessful call.
• Extend and/or move the error handling section earlier in the chapter.

[narration-sd]

hmm hmm. Nice analysis, @estebank; however, it doesn't seem in important regards to quite fit.

• the fail isn't on bad input. It occurs when further guess numbers are typed at the prompt, with return after, and appears to be because with the initial define of guess placed outside the loop, it is no longer possible to succeed after the first time it is set by read_line() and then 'shadowed' by the parse().
• (this placement is my mistake, but as mentioned is an easy one for someone used to normative considerations of scope, and anyway, it points up the problems mentioned with notation and hinting for recovery)
• to express again what I'm doing, response to first guess typed is fine, all later typed in guesses fail. I am of course typing numbers.
• I've assumed, but possibly this is wrong, that the fail occurs because read_line() can no longer enter what is typed into the now-shadowed used-to-be-string guess.
• I see your extension of the Err responses, but it's with a presupposition that we know what kinds of errors can occur. And in fact, 'invalid number' is one possibility, but while it is the error indicated, this doesn't happen to be true in the problem case, as just explained.
• Instead, it's something odd (or nil??) that guess contains after it couldn't be successfully written again by read_line(). Or at least that is the appearance of what happens...
• I think you can see the quandary here -- and this is why I feel there is something missing in Rust's consideration of either shadowing or error response when it fails.

Here are a few things I thought of after the original issue:

• shadowing itself. Interesting handling of types, it is. But if a type is 'shadowed', not converted, isn't it more sensible it should respond to either of its now-apparent types?
• and then it would be legal to define and initialize once outside the loop, as is normally efficient practice in other languages.
• The thing with RUST_BACKTRACE. I did try it, and got a truly stupendous stack trace, most of which was uninformative unless looking through Rust's compiler itself.
• Thus, I felt that RUST_BACKTRACE is really a language writer's tool, and that it's not a solution as I'd found it proposed for code issues like this (and a former one I found).
• in turn then, it seems Rust's error information should be more specific and informative -- this is the 'diagnostics' angle I felt @steveklabnik was getting at.
• Yes, this is tricky in the presence of shadowing, but that's the challenge -- and need for persons being drawn into Rust. We shouldn't have to avoid problem areas just by 'knowing better', hope you would agree.

Thanks, and Rust is interesting... Clive p.s. read_line() -- déjà vu, feel like I'm back in Pascal for a moment ;)

[estebank]

It would be useful to have a copy of the code that was causing you trouble, that way we could devise further improvements to diagnostics that could help other newcomers.

[narration-sd]

Sure -- as follows.

• Just try an initial guess, works fine
• then guess again: bogus-apparent error
• apologies for bogus Español - was just trying to practice/learn else...

use std::io;
use std::cmp::Ordering;
use rand::Rng;

fn main() {
    println!("Adivina el número!");

    let secret_number = rand::thread_rng().gen_range(1, 101);
    println!("El número secreto es: {}", secret_number);

    // here's the problem start -- initialization thus first type set outside of the loop
    let mut guess = String::new();

    loop {

        println!("Por favor ingrese su conjetura.");

        io::stdin().read_line(&mut guess)
            .expect("Error al leer la línea...");

        println!("Usted lo adivinó: {}", guess);

//        let guess: u32 = guess.trim().parse()
//            .expect("Por favor, escriba un número!");

        let guess: u32 = match guess.trim().parse() {
            Ok(num) => num,
            Err(err) => {
                println!("Número malo! {:?} : Inténtalo de nuevo...", err);
                continue
            },
        };

        match guess.cmp(&secret_number) {
            Ordering::Less => println!("¡Demasiado pequeña!"),
            Ordering::Greater => println!("¡Demasiado grande!"),
            Ordering::Equal => {
                println!("¡Tú ganas!");
                break;
            },
        }
    }
}

[narration-sd]

n.b. I've possibly made less clear by explaining too much, but the basis problem appears to be that once guess has been shadowed, it can no longer accept a string.

Then the parse fails, not because of an actual improper number, but because guess now is flagged, or contains something parse can't handle.

I did pick up your {:?} for better general error information...thanks.

[estebank]

The problem you have here is not one of shadowing, but it is indeed a wart of the api:

use std::io;
use std::cmp::Ordering;
use rand::Rng;

fn main() {
    println!("Adivina el número!");

    let secret_number = rand::thread_rng().gen_range(1, 101);
    println!("El número secreto es: {}", secret_number);

    // here's the problem start -- initialization thus first type set outside of the loop
    let mut guess = String::new();

    loop {
        guess.clear()  // <- NOTE THIS: you have to clear the `guess` buffer after every use, otherwise `read_line` will *append* to it
        println!("Por favor ingrese su conjetura.");

        io::stdin().read_line(&mut guess)
            .expect("Error al leer la línea...");

        println!("Usted lo adivinó: {}", guess);

//        let guess: u32 = guess.trim().parse()
//            .expect("Por favor, escriba un número!");

        let guess: u32 = match guess.trim().parse() {
            Ok(num) => num,
            Err(err) => {
                println!("Número malo! {:?} : Inténtalo de nuevo...", err);
                continue
            },
        };

        match guess.cmp(&secret_number) {
            Ordering::Less => println!("¡Demasiado pequeña!"),
            Ordering::Greater => println!("¡Demasiado grande!"),
            Ordering::Equal => {
                println!("¡Tú ganas!");
                break;
            },
        }
    }
}

You're appending to the buffer guess, you need to clear it. If you look at the output, if you try 10\n and then 15\n, the output will be Usted lo adivinó: 10\n15\n.

---

Filed https://github.com/rust-lang/rust/issues/56734 to deal with the env var feedback.

[narration-sd]

Fair enough -- though as you appear to recognize, the last things I'd expect from semantics of an api function named read_line are:

• that it appends to the passed-by-reference target, guess in this case -- instead of freshly writing it...
• and, that newline (and what else, \r etc. per platform) whitespace is included in the line that is read.
• I guess I could add, that a better error message from parse() would say something like 'string isn't parseable as a number', which would give a much better clue than talking about digits.
• though again, the behavior of read_line makes things very unclear in this case

Then, github just updated on-the-page, so see you've picked up on first point, though 'identifying' seems it's short of the mark as far as how this should act, unless as ever I am missing something.

Thanks, Esteban, and hope I'm contributing to development of clarity of Rust here. C.

[narration-sd]

added to, if you are reading only email...

[narration-sd]

ok, last on this I think. I had a look through old postings on Rust w/r/t read_line's behavior and related matters, not to say the doc.

I guess all this I ran into is intentional, in stdio. And also non-intuitive to many of experience.

• The point of not not clearing the buffer on read_line seems to be to allow recovery from low-level os errors which stop reads temporarily. Ok, we're relatively in OS-land rather than general user language in Rust, then.
• This non-clearing etc. may be appropriate; will have to see as I reckon Rust's intents as a language further w/r/t Web Assembly uses contemplated. I do see there is history where this acted differently, and read some of the reasons for change.
• That read_line includes the terminator of the line may be on similar intents for Rust. There may be automatic elimination of \r -- not doc'd if so; will have to try this on a Windows file, hoping for best.
• That number type conversion accepts the \n is unusual, and looks like a figment of making terminator inclusion appear 'ok'.
• String.clear() is doc'd, and can use it, yes.
• all of this seems to suggest the usefulness of a wrapper library for stdio as it sits -- or expansion of its call catalog, better.
• I see comparisons to early C++, but having been there, I can say it was much cleaner.
• the api doc such as for stdio showing functions with &self first argument references seems just plain weird -- as you don't use them that way.
• arguments I found along the way about not stating returns explicitly fall into the same category, weird. I do not grok the now-we're-controlling-cs-styles ideation about such things, which have harmed ES6 etc. through its entirely obtuse ESLint 'standards', overbearingly insisted (yes, .eslintrcs, but can you depend on them?)

Ok, got that off my chest, and maybe I have to accept Rust as a specific-corners sort of language, useful for its uses however.

Thanks for your patience to see this through, @estebank , and indeed shadowing itself seems to work as I'd thought it should in this case -- will have to read more about that in doc.

Regards, Clive

[narration-sd]

p.s. for other unenlightened pre-rusters, this would have been helpful to go through.

It does rather illustrate that I wasn't out on a limb to find things to be unusual...but also will help to get 'there'...

http://dtrace.org/blogs/ahl/2015/06/22/first-rust-program-pain/

The various issues with read_line() are right up front, and he substitutes around them. As I don't think they're very comfortable to resolve.

[Mohammad-Idrees]

The problem you have here is not one of shadowing, but it is indeed a wart of the api:

use std::io;
use std::cmp::Ordering;
use rand::Rng;

fn main() {
    println!("Adivina el número!");

    let secret_number = rand::thread_rng().gen_range(1, 101);
    println!("El número secreto es: {}", secret_number);

    // here's the problem start -- initialization thus first type set outside of the loop
    let mut guess = String::new();

    loop {
        guess.clear()  // <- NOTE THIS: you have to clear the `guess` buffer after every use, otherwise `read_line` will *append* to it
        println!("Por favor ingrese su conjetura.");

        io::stdin().read_line(&mut guess)
            .expect("Error al leer la línea...");

        println!("Usted lo adivinó: {}", guess);

//        let guess: u32 = guess.trim().parse()
//            .expect("Por favor, escriba un número!");

        let guess: u32 = match guess.trim().parse() {
            Ok(num) => num,
            Err(err) => {
                println!("Número malo! {:?} : Inténtalo de nuevo...", err);
                continue
            },
        };

        match guess.cmp(&secret_number) {
            Ordering::Less => println!("¡Demasiado pequeña!"),
            Ordering::Greater => println!("¡Demasiado grande!"),
            Ordering::Equal => {
                println!("¡Tú ganas!");
                break;
            },
        }
    }
}

You're appending to the buffer guess, you need to clear it. If you look at the output, if you try 10\n and then 15\n, the output will be Usted lo adivinó: 10\n15\n.

@estebank How is this code working after first iteration if we are shadowing the guess to u32?