3
\$\begingroup\$

To learn Rust, I tried to write a function that takes string input and then converts it to integers for a time calculation for a delay.

Is this approach ok? Have I made any mistakes? I kind of struggled with needing to do two separate error checks for the string. First for the parse result and second for the limiting the unwrapped value. I was hoping to combine the is_ok call and then unwrapped value check in one condition. Ran into some borrowing error messages and type problems.

use std::io;

fn read_time(message : &str, max : u64) -> u64 {
    let mut input = String::new();

    loop {
        println!("{}", message);
        input.clear();
        io::stdin().read_line(&mut input).unwrap();
        let num = input.trim().parse::<u64>();

        if num.is_ok() {
            let n = num.unwrap();

            if n < max {
                return n;
            }
            else {
                println!("exceeded max value of {}", max - 1);
            }
        }
        else {
            println!("Need a number above 0 and less than {}", max);
        }
    }
}

fn main() {

    println!("Waiting Sample Program");

    let hours = read_time("Enter hours: ", 24);

    let minutes = read_time("Enter minutes: ", 100);

    let wait = hours * 60 + minutes;

    println!("Going to wait {} minutes", wait);

    std::thread::sleep(std::time::Duration::new(wait * 60, 0));

    println!("done.");
}
\$\endgroup\$
3
\$\begingroup\$

Instead of checking if the parse succeeded and then unwrapping the result in two steps, you can combine the operations by using pattern matching instead. Here, an if let expression will do the job.

if let Ok(n) = num {
    // use `n` here
}

I also have some suggestions concerning style, based on the prevailing style in the community:

  1. Don't put a space between an identifier and the colon that precedes a type (message: &str rather than message : &str).
  2. Put the } and the else on the same line.

Additionally, considering the way you've written the prompt messages, it looks like there shouldn't be a newline after the prompt, so I suggest you change the println! that prints the prompt to a print!.

Here's the revised code:

use std::io;

fn read_time(message: &str, max: u64) -> u64 {
    let mut input = String::new();

    loop {
        print!("{}", message);
        input.clear();
        io::stdin().read_line(&mut input).unwrap();
        let num = input.trim().parse::<u64>();

        if let Ok(n) = num {
            if n < max {
                return n;
            } else {
                println!("exceeded max value of {}", max - 1);
            }
        } else {
            println!("Need a number above 0 and less than {}", max);
        }
    }
}

fn main() {
    println!("Waiting Sample Program");

    let hours = read_time("Enter hours: ", 24);
    let minutes = read_time("Enter minutes: ", 100);
    let wait = hours * 60 + minutes;

    println!("Going to wait {} minutes", wait);

    std::thread::sleep(std::time::Duration::new(wait * 60, 0));

    println!("done.");
}

If you want to combine the result of parse with the range checking in a single operation, that's possible too. First, let's look at the "lazy" way, where both error situations will produce the same error message:

fn read_time(message: &str, max: u64) -> u64 {
    let mut input = String::new();

    loop {
        print!("{}", message);
        input.clear();
        io::stdin().read_line(&mut input).unwrap();
        let num = input.trim().parse::<u64>();

        if let Some(n) = num.ok().and_then(|n| if n < max { Some(n) } else { None }) {
            return n;
        }

        println!("Need a number above 0 and less than {}", max);
    }
}

Here, I'm using ok to convert the Result to an Option, because I'm too lazy to define an error type that unifies the parse error and our new out-of-range condition. Then I'm using and_then to perform the range validation only if the parse succeeded. The closure returns Some(n) if the value is in range, or None if the value is out of range.

Instead of using and_then, we could also use the filter method provided by the option-filter crate. It takes a closure that returns a bool, and returns None if the closures returned false, or the original value otherwise.

if let Some(n) = num.ok().filter(|n| n < max) {
    return n;
}

And now, let's not be lazy and code this up properly. We'll need to introduce a type that can represent either a parse error, or an out-of-range condition.

use std::num::ParseIntError;

enum ReadTimeError {
    ParseError(ParseIntError),
    ValueOutOfRange,
}

impl From<ParseIntError> for ReadTimeError {
    fn from(e: ParseIntError) -> Self {
        ReadTimeError::ParseError(e)
    }
}

fn read_time(message: &str, max: u64) -> u64 {
    let mut input = String::new();
    let check_in_range =
        |n| if n < max { Ok(n) } else { Err(ReadTimeError::ValueOutOfRange) };

    loop {
        print!("{}", message);
        input.clear();
        io::stdin().read_line(&mut input).unwrap();
        let num = input.trim().parse::<u64>();

        match num.map_err(ReadTimeError::from).and_then(check_in_range) {
            Ok(n) => return n,
            Err(ReadTimeError::ParseError(..)) => {
                println!("Need a number above 0 and less than {}", max);
            }
            Err(ReadTimeError::ValueOutOfRange) => {
                println!("exceeded max value of {}", max - 1);
            }
        }
    }
}

Here, we have three cases to handle, so I'm using a match expression rather than an if let expression. I'm taking advantage of the fact that patterns can match "deeply" to match on two levels of enums at the same time. Result also has an and_then method, and naturally, the closure must return a Result. I moved the closure out of line to keep the expression being matched on short enough.


Now, it's up to you to decide whether combining the two validations is worth the additional complexity or not! :)

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.