3
\$\begingroup\$

Another code for exercism, It is to convert Vec of u32 from one base to another base. Please help to make it more idiomatic, Also if this type of question is welcomed here or not.

#[derive(Debug, PartialEq)]
pub enum Error {
    InvalidInputBase,
    InvalidOutputBase,
    InvalidDigit(u32),
}


pub fn convert(number: &[u32], from_base: u32, to_base: u32) -> Result<Vec<u32>, Error> {


    if from_base < 2 {
        return Err(Error::InvalidInputBase);
    }

    if to_base <2  {
        return Err(Error::InvalidOutputBase);
    }

    if number.iter().any(|&x| x > 0 && x >= from_base ){
        return Err(Error::InvalidDigit(from_base));
    }

    let  res: u32 = number.iter().rev().enumerate().map(|(index, x)| x*from_base.pow(index as u32)).sum();
    Ok(convert_back(res, to_base))

}



fn convert_back(number: u32, base: u32) -> Vec<u32>{

    let mut res: Vec<u32> = Vec::new();
    let mut inter = number as f64;

    loop {
        inter = inter as f64/base as f64;
        res.push((inter.fract()*base as f64).round() as u32);
        if inter as u32 ==0{
             break
             }

        inter = inter.trunc();
        }

        res.reverse();
        res

}

Some tests


fn decimal_to_binary() {
    let input_base = 10;
    let input_digits = &[4, 2];
    let output_base = 2;
    let output_digits = vec![1, 0, 1, 0, 1, 0];
    assert_eq!(
        ayb::convert(input_digits, input_base, output_base),
        Ok(output_digits)
    );
}

fn trinary_to_hexadecimal() {
    let input_base = 3;
        let input_digits = &[1, 1, 2, 0];
        let output_base = 16;
        let output_digits = vec![2, 10];
    assert_eq!(
       convert(input_digits, input_base, output_base),
        Ok(output_digits)
    );
}

fn hexadecimal_to_trinary() {
    let input_base = 16;
    let input_digits = &[2, 10];
    let output_base = 3;
    let output_digits = vec![1, 1, 2, 0];
    assert_eq!(
        convert(input_digits, input_base, output_base),
        Ok(output_digits)
    );
}

fn invalid_positive_digit() {
    let input_base = 2;
    let input_digits = &[1, 2, 1, 0, 1, 0];
    let output_base = 10;
    assert_eq!(
        convert(input_digits, input_base, output_base),
        Err(Error::InvalidDigit(2))
    );
}

fn output_base_is_zero() {
    let input_base = 10;
    let input_digits = &[7];
    let output_base = 0;
    assert_eq!(
        convert(input_digits, input_base, output_base),
        Err(Error::InvalidOutputBase)
    );
}

```
\$\endgroup\$
1
  • \$\begingroup\$ It's considered bad form to edit the code in your questions after they're posted. The changes you've made are good, but I'd already started writing an answer! \$\endgroup\$
    – wizzwizz4
    Apr 30, 2020 at 14:05

1 Answer 1

1
\$\begingroup\$

Style

These are in order of me thinking of them; not in order of first appearance. Sorry!

#[test]

If you mark your tests with #[test], then running cargo test will run them. If you also mark them with #[cfg(test)], then they'll only be compiled in test mode.

Unneeded collect()

cargo clippy is an invaluable tool. Here, it found an optimisation!

if !number.iter().filter(|&x| *x > 0 && *x >= from_base ).collect::<Vec<_>>().is_empty(){
    return Err(Error::InvalidDigit(from_base));
}

It points out that .collect::<Vec<_>>().is_empty() should be replaced with .next().is_none():

if !number
    .iter()
    .filter(|&x| *x > 0 && *x >= from_base)
    .next()
    .is_none()
{
    return Err(Error::InvalidDigit(from_base));
}

The compiler's probably smart enough to turn this into efficient code,[citation needed] but this code can be made clearer:

if number.iter().any(|&x| x > 0 && x >= from_base) {
    return Err(Error::InvalidDigit(from_base));
}

Unnecessary casting

inter = inter as f64/base as f64;

can be

inter /=  base as f64;

and you can get rid of all of the base as f64 bits by putting

let base = base as f64;

at the top of convert_back.

After a quick pass through cargo fmt to clean up the errant spacing and expand the iterator chain to separate lines, this all looks pretty good.

Algorithm

Your algorithm is, sadly, more limited than it looks. Your use of as is a little risky, as casting to a smaller integer truncates it. However, that won't get to be a problem, because as soon as the from_base.pow ends up with a number bigger than u32 can store, it'll panic.

You need some kind of "too big" Error variant if you want to handle this gracefully; you could check whether number.len() > u32::MAX (otherwise as's truncation could cause issues), and then check whether all the .pow calls are guaranteed to work (perhaps with .checked_pow?), and then whether adding all the digits together will exceed the bounds of u32.

Alternatively, you could make the algorithm convert via base 2³² instead of u32s, which would resolve the issue.

\$\endgroup\$

Your Answer

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

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