4
\$\begingroup\$

There is a function that returns digits as vector of given number. Is it idiomatic way to solve this problem in Rust? Can you suggest more concise solution?

#[allow(dead_code)]
fn num_digits(num: i32) -> Vec<i32> {
    let mut x = num;
    let mut result: Vec<i32> = Vec::new();

    loop {
        result.push(x % 10);
        x /= 10;
        if x == 0 {break}
    }

    result.reverse();
    result
}

#[test]
fn digits() {
    assert_eq!(num_digits(0), [0]);
    assert_eq!(num_digits(-1), [-1]);
    assert_eq!(num_digits(-123), [-1, -2, -3]);
    assert_eq!(num_digits(1), [1]);
    assert_eq!(num_digits(12), [1, 2]);
    assert_eq!(num_digits(456464), [4, 5, 6, 4, 6, 4]);
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ I have writtent a crate for that: crates.io/crates/digits_iterator \$\endgroup\$
    – Boiethios
    Commented Aug 16, 2019 at 8:25
  • \$\begingroup\$ Interesting @FrenchBoiethios. Your iterator impl looks very much like my unfold solution, but I don’t grok how you’re handling the 0 case. \$\endgroup\$
    – RubberDuck
    Commented Aug 18, 2019 at 23:41

2 Answers 2

3
\$\begingroup\$

First thing's first, you probably don't want to leave the #[allow(dead_code)] option on. If the method is meant to be part of the public API, make it pub.

It's also best to split your tests up into different test functions so that they all run every time. The way you've written it, when one fails, the tests stop and don't execute the other examples. In order to effectively refactor the code, I had to split them up.

#[test]
fn zero() {
    assert_eq!(num_digits(0), [0]);
}

#[test]
fn single_digit_negative() {
    assert_eq!(num_digits(-1), [-1]);
}

#[test]
fn triple_digit_negative() {
    assert_eq!(num_digits(-123), [-1, -2, -3]);
}    

#[test]
fn single_digit_positive() { 
    assert_eq!(num_digits(1), [1]);
}        

#[test]
fn double_digit_positive() {
    assert_eq!(num_digits(12), [1, 2]);
}

#[test] 
fn large_positive() {
    assert_eq!(num_digits(456464), [4, 5, 6, 4, 6, 4]);
}

With that out of the way, let's talk about idiomatic Rust. In Rust, it is far more common to use functional style iterators rather than imperative loops. However, I've found that sometimes it's better to just write the imperative loop rather than jump through hoops to write the equivalent map/reduce. I would say that your loop is absolutely fine, suited to its purpose, and avoids converting the numbers to strings. Although it leaves me wishing that Rust had do {} while cond syntax. That's a failing of the language though, not one of yours. The break is the right call here and preferable to this "trick" you sometimes see.

while {
    result.push(x % 10);
    x /= 10;
    x != 0
} { }

I prefer the break you used over abusing this quirk of the language.

If you do wish to pursue an approach using iterators though, you can. You see the fold function takes a sequence and returns a single value via reduction. There is also an unfold function which does the opposite. It takes a seed value and produces a sequence from it, which is exactly what you're doing here. Rust doesn't have an unfold function, but it does have std::iter::from_fn, which is very close in functionality.

fn num_digits(num: i32) -> Vec<i32> {
    /*
     * Zero is a special case because
     * it is the terminating value of the unfold below,
     * but given a 0 as input, [0] is expected as output.
     * w/out this check, the output is an empty vec.
     */
    if num == 0 {
        return vec![0];
    }

    let mut x = num;
    let mut result = std::iter::from_fn(move || {
        if x == 0 {
            None
        } else {
            let current = x % 10;
            x /= 10;
            Some(current)
        }
    })
    .collect::<Vec<i32>>();

    result.reverse();
    result
}

Now, is that actually any better? I, personally, don't feel like it is. It could be if we changed the method to return the iterator instead of the vector. If we did that, then it could be lazily evaluated instead of calculating every item in the sequence up front. That could make this alternative approach worthwhile if we were dealing with very large integers that would generate very large vectors, but that's a pretty big if.

\$\endgroup\$
1
\$\begingroup\$

I would call this "more idiomatic" but I'm still not satisfied with it. Rust uses Iterators in a lot of ways, e.g. look at this implementation:

pub fn num_digits(num: i32) -> Vec<i32> {
    let mul = if num < 0 { -1 } else { 1 };
    num.to_string()
        .chars()
        .filter_map(|x| x.to_digit(10))
        .map(|x| (x as i32) * mul)
        .collect()
}

The mul variable is just for determining if you want negative or positive numbers.

Because i32 implements the Display trait you can call to_string on it, which will give you a String representation of the i32.
Next is just iterating of the characters of the String and trying to convert them to a digit. - will be ignored, because it is not a digit and filter_map will filter all Results, that will return a Err. The next thing is to convert the numbers to a i32 (because to_digit gives an u32) and either making them negative if the original number was negative or staying positive.

collect will put all the numbers into a collection, in this case a Vec because the type can be deduced by Rust (because of the return type)

\$\endgroup\$
2
  • 1
    \$\begingroup\$ I thought about something like this, but I don't want to store string representation of number in memory. \$\endgroup\$
    – Stexxe
    Commented Aug 16, 2019 at 10:06
  • \$\begingroup\$ I'm upvoting for the use of iterators, but it doesn't feel great to turn it into a string. \$\endgroup\$
    – RubberDuck
    Commented Aug 18, 2019 at 3:00

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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