Error when using read_line in a loop

help
1

This skeleton program gives an error (invalid digit) on the second pass through the loop, when the digit is correct.

use std::io;

fn main() {
	let mut inp = String::new();
	loop {
		println!("What do you want to do?");
		println!(" 1. Some option");
		println!(" 2. Another option");
		// ...... further options
		println!(" 0. Exit");

		io::stdin()
			.read_line(&mut inp)
			.expect("Failed to read line");
		let choice: u32 = inp.trim().parse().expect("Not a number!");
		if choice == 0 { println!("Goodbye!"); break }
	}
}
2

You can debug it by inserting a dbg!(inp) statement inside the loop.
I suspect you should be clearing inp on each iteration, since otherwise new input is concatenated to the old.

2 Likes
3

I think Rust API isn't intuitive here, it is my typical error even after 2 years of Rust programming.

4

You need to empty the inp variable before reading into it again, because String is actually a StringBuffer, and when you call read_line, the new content is directly appended into it.

5

This should work:

use std::io;
fn main() {
    let mut inp = String::new();
    loop {
        println!("What do you want to do?");
        println!(" 1. Some option");
        println!(" 2. Another option");
        // ...... further options
        println!(" 0. Exit");
        
        io::stdin().read_line(&mut inp).expect("Failed to read line");
        let choice: u32 = inp.trim().parse().expect("Not a number!");
        inp = String::new();
        match choice {
            0 => { println!("Goodbye!"); break; },
            _ => {}
        }
    }
}
6

That's just because String in rust itself is mutable unlike in other languages.

7

Thanks for all the replies. It does work with the change, but I must say that read_line appending to the variable is a real gotcha.

8

A tiny thing here would be to change

inp=String::new();

to

inp.clear();

Here, it doesn’t really matter that much (I guess it is a very tiny performance hit), but in general it would be a better choice because the first allocates a new String (and deallocates the old one) while the latter keeps the allocation and just “removes” the old content.

2 Likes
9

I wonder if inp=String::new();or let mut inp=String::new();in a loop actually allocates a new string on each iteration, or if the compiler is smart enough to reuse the previous allocation. Have you tested it actually, or do you just know it?

10

That the Sting::clear() option is in general better? No, but I am convinced that reusing an allocation instead of creating a new one is probably in general faster than making a new one and deallocating the old one. I am - in this case - not 100% sure whether it is applicable, but for larger Strings it is very definitely.

11

Sorry, my remark might not have been clear enough, and the answer might indeed be obvious. Of course clear() is better. But when we need a string only within a loop, it is not that nice that we have to declare the string already outside the loop, and then reuse it with clear(). So I was wondering if latest compilers might be smart enough to do that for us, when we just use new() in the loop. But well, then the compiler would actually replace a function call with a different one, so I guess that will not happen. But the compiler might at least optimize the heap allocation, by just reusing the same character buffer in each loop iteration, instead of requesting a new buffer from the OS for each loop iteration. I have never tested this, and compared the actual performance impact.

12

This is one of those things I've always hoped compilers would do, but I can't imagine they can. The compiler can't know whether it's important or not to perform the actual allocation on each iteration. The allocator may have side-effect, like statistics & diagnostics that the developer expect to be triggered on each source-level invocation.

If you allow me to hyperbole: Imagine someone hooks the breathing cycle trigger into a memory allocator in a heart/lung machine:

loop {
  let m = Box::new(42); // breathe in
  sleep(AVERAGE_INHALE);
  drop(m); // breathe out
  sleep(AVERAGE_EXHALE);
}

Now obviously no one would do that, but that's not the point -- the point is that the compiler can't know that.

3 Likes
13

Note that String::new() never allocates anyways. The allocation would happen as part of read_line (which ofc is still part of the loop). In my anecdotal experience, this would probably be optimized out (I've seen it happen in several similar situations).

1 Like
14

Anecdotal like the rest, but in my experience you can't count on it optimizing out, and I'm not alone. So if you care reuse your buffers.

(For something waiting on human input, it's not going to matter.)

4 Likes