Signed / unsigned conversions

code review
1

I think this function from my music player illustrates that signed <-> unsigned conversions can be tricky and I would like suggestions how to make this "idiomatic"...

/// Render an ascii bar graph of size `w` * `h` into `target`
fn print_bars(bars: &[f32], target: &mut [char], w: usize, h: usize) {
    const C: [char; 9] = ['█', '▇', '▆', '▅', '▄', '▃', '▂', '▁', ' '];
    for x in 0..bars.len() {
        let n = (bars[x] * (h as f32 / 5.0)) as i32;
        for y in 0..h {
            let d = ((h - y) * 8) as i32 - n;
            target[x + y * w] = C[d.clamp(0, 8) as usize];
        }
    }
}
2

I'd argue that if each of the as casts is actually necessary in order to get the correct output, then this is the idiomatic solution even if it looks noisy.

You could replace the as casts with .into() but that wouldn't really change the overall level of noise.

1 Like
3

I don't see any practical uses of into() in this code though, and I was wondering about that.

My thinking here is that the "dangerous" conversions is signed -> unsigned as it can truncate values so you want to do that as late (and as seldom) as possible,

4

It doesn't seem to be necessary to use signed numbers here at all. It is really unclear what exactly your code is supposed to be doing, but by guessing the input data based on the magic numbers, you can do this and remove all but one cast:

fn print_bars(bars: &[f32], target: &mut [char], w: usize, h: usize) {
    let chars = ['█', '▇', '▆', '▅', '▄', '▃', '▂', '▁', ' '];
    for (x, &bar_x) in bars.iter().enumerate() {
        let n = (bar_x * (h as f32 / 5.0)) as usize;
        for y in 0..h {
            let d = (h - y) * 8 - n;
            target[x + y * w] = chars[d.clamp(0, 8)];
        }
    }
}
5

No, the let d subtraction can be negative. It calculates if it is above or below the top of the bar, to know which character to print.

6

If it can usefully be negative, then why are you clamping it to 0 from below? Isn't

(signed_small - signed_large).clamp(0, MAX)

just the same as

unsigned_small.saturating_sub(unsigned_large).min(MAX)

?

7

It is usefully negative, then I clamp it to get the correct character.
Yes, I could replace it with a saturating_sub() (I suppose), didn't know about that method. I am not sure it is an improvement though.

8

It's an improvement because you can then completely get rid of signed <-> unsigned conversions, like this.

1 Like
9

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.