4
\$\begingroup\$

Once again, I am reinventing the wheel here to understand the fundamentals of rust. In a previous question I requested feedback on my function that performed a hexadecimal to binary conversion. In this snippet, I am seeking some constructive feedback on converting from binary to hexadecimal.

fn main() {

    let hexadecimal_value = convert_to_hex_from_binary("10111");
    println!("Converted: {}", hexadecimal_value);
}

fn convert_to_hex_from_binary(binary: &str) -> String {
    let padding_count = 4 - binary.len() % 4;

    let padded_binary = if padding_count > 0 {
        ["0".repeat(padding_count), binary.to_string()].concat()
    } else {
        binary.to_string()
    };

    let mut counter = 0;
    let mut hex_string = String::new();
    while counter < padded_binary.len() {
        let converted = to_hex(&padded_binary[counter..counter + 4]);
        hex_string.push_str(converted);
        counter += 4;
    }

    hex_string
}

fn to_hex(b: &str) -> &str {
    match b {
        "0000" => "0",
        "0001" => "1",
        "0010" => "2",
        "0011" => "3",
        "0100" => "4",
        "0101" => "5",
        "0110" => "6",
        "0111" => "7",
        "1000" => "8",
        "1001" => "9",
        "1010" => "A",
        "1011" => "B",
        "1100" => "C",
        "1101" => "D",
        "1110" => "E",
        "1111" => "F",
        _ => "",
    }
}


\$\endgroup\$
3
+50
\$\begingroup\$

A couple of things:

  1. Any time you have a function which could fail, it should return an Option<T>. Ask yourself, if someone calls convert_to_hex_from_binary("foobar") and gets back "", is that reasonable? They will need to manually check that their input makes sense, or that the output makes sense every time. Static checking of these errors is part of the joy of Rust.

With that it mind, change to_hex like so:

fn to_hex(b: &str) -> Option<&str> {
    match b {
        "0000" => Some("0"),
        "0001" => Some("1"),
        "0010" => Some("2"),
        "0011" => Some("3"),
        "0100" => Some("4"),
        "0101" => Some("5"),
        "0110" => Some("6"),
        "0111" => Some("7"),
        "1000" => Some("8"),
        "1001" => Some("9"),
        "1010" => Some("A"),
        "1011" => Some("B"),
        "1100" => Some("C"),
        "1101" => Some("D"),
        "1110" => Some("E"),
        "1111" => Some("F"),
        _ => None,
    }
}
  1. You are iterating over a known range of values to get counter. Instead of using a while loop, you can use a range. Since you're now returning None from to_hex you can now shortcut returning if something is wrong with None.

Your inner loop now looks like this:

let mut hex_string = String::new();
for counter in (0..padded_binary.len()).step_by(4) {
    match to_hex(&padded_binary[counter..counter + 4])
    {
        Some(converted) => hex_string.push_str(converted),
        None => return None
    };
}

Your function signature and return type also need to match Option<String>:

fn convert_to_hex_from_binary(binary: &str) -> Option<String> {
    ...
    Some(hex_string)
}
  1. Your padded_binary definition can be simplified, since repeating "0" zero times is exactly the same as not repeating it.

The definition is simply:

let padded_binary = ["0".repeat(padding_count), binary.to_string()].concat();

A couple of non Rust specific things:

  1. Consider writing some tests.

  2. String representation of binary numbers often begin with '0b'. For example 0b1101 == 13. You might want to consider checking for this prefix on the input string and trimming it.

  3. You may have already thought about this, but consider if you want to trim whitespace or leave it to the function caller.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the great review. The use of Option is definitely something that makes a great difference. \$\endgroup\$ – sc_ray Sep 18 at 19:59

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.