0
\$\begingroup\$

I wrote these lines of rust yesterday. It turns 27 bits from a 32 bit integer into a 3x9 byte array, but my gut feeling is this should be done without having a mutable variable ans.

pub fn u32_to_preformated_vector(image:u32) ->[[u8;9];3] {
  let mut ans:[[u8; 9]; 3]= [[0,0,0,0,0,0,0,0,0],[0,0,0,0,0,0,0,0,0],[0,0,0,0,0,0,0,0,0]];
  let image = !image;
  for i in 0..27 {
     ans[i/9][i%9] = 1 & (image >> i) as u8;
  }
  ans
}

What would make the cleanest code here? A macro or some iterator magic?

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

Please also see also the code here: https://play.rust-lang.org/?version=stable&mode=debug&edition=2018&gist=574d96b16d82c5816ebc35c5d88acda2

The name of the function u32_to_preformated_vector let me assume I get something else that what I'm actually get. So I think the naming could improve:

  • the function actually does not return a vector, but an array;
  • the function does invert the input bits. That's completely invisible in the name.
  • You might even indicate the bit order you handle the conversion in, as not everybody might expect to get the least significant bit/the least significant nonet first.

Keeping the mutable array, I came to this code:

pub fn u32_to_preformatted_array(image: u32) -> [[u8; 9]; 3] {
    let mut result = [[0u8; 9]; 3];
    let inverted = !image;
    for n in 0..3 {
        let nonet = inverted >> n * 9;
        for m in 0..9 {
            result[n][m] = 1 & (nonet >> m) as u8;
        }
    }
    result
}

I thought it would help if I bind the inverted image to a new name (inverted). That way in the for loop the reader sees that it is a different value to the one that was passed to the function as the argument.

I also converted the single for loop to two nested for loops. While nesting these loops in general might cause performance problems, it does no harm here, as the number of iterations in the inner code keep the same (27). That way I think the index handling is clearer than doing a divide and a remainder to calculate them. It might also make the bit order more obvious.

The initial value of the array can be initialized in a compacter way as all values in it are the same (0u8 in this case).

I don't think having a mutable array inside the function is a big deal. While I very much go for immutability in general, here the context within which mutation is possible is quite limited. As this is the place where the value gets built, there is no danger of it being used at a different place. From the point you return the value, it will be immutable by the declaration of your function.

I was also trying to get rid of this single mut by using iterators. It's quite easy as long as you change the return type of the function from arrays to vectors:

pub fn u32_to_preformatted_vec(image: u32) -> Vec<Vec<u8>> {
    let inverted = !image;
    (0..3)
        .map(|i| (inverted >> 9 * i) & 0b111_111_111) // `&` not strictly needed
        .map(|nonet| (0..9).map(|i| 1 & (nonet >> i) as u8).collect())
        .collect()
}

In general I have the impression working in a map/reduce style works a lot better with Vec than on arrays. You can also convert the Vec to a slice, but the conversion to an array is harder and often is done using functions like .clone_from_slice() which require a previously allocated array that is mutable again. I also tried to get a slice for the Vec and then use the TryFrom trait to convert it to an array. While I got this working for single dimensional array, I didn't get it to work for the multidimensional array [[u8; 9]; 3].

There is also rarely a need to go from a Vec to an array when designing the solution. Vec is just a little bit of code around an array (that's why to can get a slice from it so easily). So you don't really save anything when using an array.

Thinking about the whole problem as such, I wonder what's the reason for this conversion. You're blowing a 4 byte value up to using 27 bytes of memory. That way you increase the memory usage of your program, and I don't know what you get for it. I don't think that it will actually improve performance because you loose locality when accessing the data. The caching in your computer will not work as good as by using the original 4 byte value.

If the goal you try to get from the conversion is improved readability, I think you might try to define helper functions to get (or set) the individual bits. The calculations are actually quite easy, are performed in the CPU registers and can be inlined by the compiler. That should be quite fast … faster than converting the data to this two dimensional array which requires a lot of access to the computer's memory because it cannot be done in registers.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I will comment on this more later but for now- yes the whole code seems strange but the reason for this crazy conversion is to use my small u32 encoded images in the microbit crate which takes a 3x9 array as argument. \$\endgroup\$ – Simson Apr 17 at 1:08

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.