Factorial number in Rust for learning purposes

Question

I'm currently trying to learn the basics of the Rust programming language, to do this my first piece of code after the classic 'Hello World' was a simple recursive Factorial function.

This is the code:

use std::io;

fn factorial(num: i64) -> i64 {
    return if num == 1 {
        1
    }else{
        num * factorial(num - 1)
    }
}

fn trim_newline(s: &mut String){
    if s.ends_with('\n'){
        s.pop();
        if s.ends_with('\r'){
            s.pop();
        }
    }
}

fn main() {
    let mut user_input = String::new();
    println!("Welcome to rust-factorial!");
    loop {
        user_input.clear();
        println!("Please enter a number:");
        match io::stdin().read_line(&mut user_input) {
            Ok(_) => (),
            Err(_e) => {
                println!("error: {}", _e);
                continue;
            },
        };
        let num: i64 = match user_input.trim().parse::<i64>() {
            Ok(n) => n,
            Err(_e) => {
                trim_newline(&mut user_input); // remove trailing newline
                println!("'{}' is not a valid number, full error: {}", user_input, _e);
                continue;
            },
        };
        println!("{}! is equal to {}", num, factorial(num));
    }
}

I am aware of one major issue with my current approach which is that the biggest number this program can calculate is the 20th number of the sequence. Asking for a number bigger than 20 results in the following error on my machine:

Please enter a number:
24
thread 'main' panicked at 'attempt to multiply with overflow', src\main.rs:7:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace.
error: process didn't exit successfully: `target\debug\rust-factorial.exe` (exit code: 101)

I'd like to know a few things:

• How can I change this code to be more "rust-like"
  • (by that I mean 'pythonic' but for rust)
• What would the rust approach be for solving the overflow that happens with numbers past 20?
  • (I could either hard-code the limit or something to that effect but I'm curious whether or not there is a guideline about this from a Rust perspective)
• Any and all comments on code quality, readability and potentially efficiency

Answer 1

Bugs and types

You should think some about what types you want to have as input and output for factorial. Negative numbers won't ever work as input, and the output won't be negative, so you should consider using an unsigned integer, like u64. Even doing this, though, there's one input that leads to an infinite loop. See if you can find it and fix the function!

Cleaning up the error handling

match io::stdin().read_line(&mut user_input) {
    Ok(_) => (),
    Err(_e) => {
        println!("error: {}", _e);
        continue;
    }
};

A variable with a leading underscore tells readers (and linters) that the variable isn't going to be used. Since we use it (with the println!), we should just call it e.

This error message should probably be in stderr, rather than stdout. This can be done just by changing it to eprintln! instead.

When only one branch of a match actually does something, if let is both easier to write and understand. So in all, I'd change this part to

if let Err(e) = io::stdin().read_line(&mut user_input) {
    eprintln!("error: {}", e);
    continue;
}

---

let num: i64 = match user_input.trim().parse::<i64>() {
    Ok(n) => n,
    Err(_e) => {
        trim_newline(&mut user_input); // remove trailing newline
        println!("'{}' is not a valid number, full error: {}", user_input, _e);
        continue;
    },
};

Same thing with _e vs e. You're using the variable, so it shouldn't start with an underscore. Also the same thing with eprintln!. Errors should be printed to stderr.

Since both branches of the match are used, if let is less useful. However, you might check out the answers to this question for some ideas for making this look better. For a single use like this, you're probably fine, but if you do this "unwrap or continue" behavior a lot, it can help to encapsulate it.

The i64 annotations are unnecessary. The compiler has enough information to conclude that the type of num is i64 since it's fed into factorial. If you want to keep an annotation for explicitness, that's fine, but only one of num: i64 and parse::<i64> is needed for that.

trim_newline seems to be unnecessary. You're parsing user_input.trim() anyway, so just show that to the user if there's a parsing error.

let num = match user_input.trim().parse() {
    Ok(n) => n,
    Err(e) => {
        eprintln!("'{}' is not a valid number, full error: {}", user_input.trim(), e);
        continue;
    }
};

Using standard tools for formatting and linting

One thing that goes a long way with making your code more readable and idiomatic is running cargo fmt. You may need to install it with rustup component add rustfmt.

Clippy is another standard tool for catching errors and making your code more idiomatic. It can similarly be installed with rustup using rustup component add clippy. Running it (with cargo clippy) on your program, we get the warning

warning: unneeded `return` statement
 --> src/main.rs:4:5
  |
4 | /     return if num == 1 {
5 | |         1
6 | |     }else{
7 | |         num * factorial(num - 1)
8 | |     }
  | |_____^
  |
  = note: `#[warn(clippy::needless_return)]` on by default
  = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#needless_return
help: remove `return`
  |
4 |     if num == 1 {
5 |         1
6 |     }else{
7 |         num * factorial(num - 1)
8 |     }
  |

Since the return statement is the last thing in the function, it's not needed since the last expression in the block is always implicitly returned. You can simply remove the return.

Some advice on overflows

The factorial function grows superexponentially, so any fixed-width integer type is going to quickly overflow. Even switching to u128 only gets you up to an input of 34. The way I see it, you have two choices. Either

• Embrace this. Return an Option (or better yet, a Result) to indicate that an overflow occurred. Using something like u64::checked_mul would be helpful for that.
• Avoid overflow completely (barring memory limits) by using some sort of BigInt. The crate num_bigint has an implementation with everything you'd need here, including parsing, printing, multiplying and subtracting. You should be able to replace i64 with BigUint (or BigInt) with only a few conversions from integer literals.