Parse input string into vector of i16 in Rust

Question

I'm new to Rust and I'm working through the exercises found at the bottom of this page. The following code parses a space deliminated input string from the user into a Vec<i16>. If the input string is invalid, the code loops and prompts again. If the string is valid, it prints out the debug value of the Vec<i16> and prompts again.

The code works, but I feel there is a more idomatic Rust way to deal with this. Particularly, in the get_input function's looping and how its assigning the return value. Behold.

use std::io::Write;

fn main() {
    while let Some(ary) = get_input() {
        println!("{:?}", ary);
    }

    println!("Peace!");
}

fn get_input() -> Option<Vec<i16>> {
    let mut out: Option<Vec<i16>> = None;
    let mut valid = false;
    while !valid {
        // Get user input.
        print!("Enter series-> ");
        std::io::stdout().flush().unwrap();
        let mut input = String::new();
        std::io::stdin().read_line(&mut input).unwrap();

        // Parse it.
        match input.trim() {
            "q" => {
                valid = true;
                out = None;
            }
            _ => {
                let parsed = parse_input(input);
                if let Ok(_) = parsed {
                    out = Some(parsed.unwrap());
                    valid = true;
                }
            }
        }
    }
    out
}

fn parse_input(input: String) -> Result<Vec<i16>, std::num::ParseIntError> {
    input
        .split_whitespace()
        .map(|token| token.parse::<i16>())
        .collect::<Result<Vec<i16>, _>>()
}

Answer 1

I appreciate that you have formatted your code in the current idiomatic Rust style; thanks!

1. There's no reason to set the type of the out variable, type inference will handle it.

2. There's no need for the out or valid variables at all. Switch to an infinite loop and return from it when you need to.

3. It's likely overkill, but you are allocating and freeing the string for each loop. You could instead pull it outside of the loop to reuse it some, or pull the whole thing into a structure and reuse it for the entire program. You will need to ensure you clear the string before each use.

4. Don't unwrap inside of an if let. Instead, bind the result to a variable when doing the pattern matching and use it inside the block. (Clippy also tells you this, in a slightly different manner)

5. There's no benefit to taking a String in parse_input; you don't reuse the allocation. Accept a &str instead. (Clippy also tells you this)

6. You don't need any of the turbofish in parse_input; type inference knows what to do based on the return type.

use std::io::Write;

fn main() {
    while let Some(ary) = get_input() {
        println!("{:?}", ary);
    }

    println!("Peace!");
}

fn get_input() -> Option<Vec<i16>> {
    loop {
        // Get user input.
        print!("Enter series-> ");
        std::io::stdout().flush().unwrap();
        let mut input = String::new();
        std::io::stdin().read_line(&mut input).unwrap();

        // Parse it.
        match input.trim() {
            "q" => return None,
            input => {
                if let Ok(numbers) = parse_input(input) {
                    return Some(numbers);
                }
            }
        }
    }
}

fn parse_input(input: &str) -> Result<Vec<i16>, std::num::ParseIntError> {
    input
        .split_whitespace()
        .map(|token| token.parse())
        .collect()
}

Answer 2

I don't know Rust too well, so take this review with a grain of salt. That being said, your get_input can be heavily simplified if you return. That way we don't need to remember whether the parsed input was valid: if it's valid we just return:

fn get_input() -> Option<Vec<i16>> {
    loop {
        // Get user input.
        print!("Enter series-> ");
        std::io::stdout().flush().unwrap();
        let mut input = String::new();
        std::io::stdin().read_line(&mut input).unwrap();

        // Parse it.
        match input.trim() {
            "q" => return None,
            _ => {
                let parsed = parse_input(input);
                if let Ok(out) = parsed {
                    return Some(out);
                }
            }
        }
    }
}

Note that this prevents some errors that were possible before. For example you could have missed to set out and accidentally returned None instead of Some, e.g.

    // ...
    _ => {
        let parsed = parse_input(input);
        if let Ok(_) = parsed {
            valid = true;
            // whoops forgot out
        }
    }

This erroneous state cannot happen now.

Also note that we now have a simple loop. We don't have any mut bindings, only a single if let is left. Also note that we don't need to unwrap parsed: if parsed is an Ok(out), we can use that out.

That being said, you should add an error message after the match, e.g.

match input.trim() {
    // same as above
}
println!("Your input could not get parsed. Please write space-separated numbers or 'q' to quit");