Is there a more idiomatic way of doing this?

help
1

I'm trying to get the value just before an index from an array, and handle the case where the index is at the beginning by returning zero instead. These are the things I've tried so far, both of which work:

let prev = if i == 0 { 0 } else { values[i - 1] };
let prev = i.checked_sub(1).map(|i| values[i]).unwrap_or(0);

They both feel kind of ugly to me, but nothing else comes to mind that makes it cleaner. What is the idiomatic way of solving this problem?

2

I think the first option is quite clear by itself. I'd have to think for a solid minute to figure out what was going on with the second.

1 Like
3

I'd like to see the use of match rather than if for this:

    let prev = match i {
        0 => 0,
        _ => values[i - 1],
    };

It says clearly what the intention is.
It stops cargo fmt squashing it up into a single line that is harder to read.

5 Likes
4

Does not work:

thread 'main' panicked at 'attempt to subtract with overflow',...
5

I think the second version is more expressive. I like that subtraction and checking happens in the same operation (checked_sub) instead of i - 1 relying on a previously checked condition. Using if let could perhaps be more readable because you can give a better name to the index variable:

let prev = if let Some(previous_index) = i.checked_sub(1) {
    values[previous_index]
} else {
    0
};

values.get(i - 1).unwrap_or(0) would be perfectly idiomatic if it worked, but sadly it doesn't because indexing uses usize.

6

I think I'd prefer the match version of @Riateche's

let prev = match i.checked_sub(1) {
    Some(previous_index) => values[previous_index],
    None => 0
};
6 Likes
7

Why add all that syntactic noise when all one needs is:

    let prev = match i {
        0 => 0,
        _ => values[i - 1],
    };
5 Likes
8

i like this one more

but i would use map_or()

let prev = i.checked_sub(1).map_or(0, |i| values[i]);
4 Likes
9

Looking again, I guess I like yours the best once you fix the bug (you never actually subtract one from i :slight_smile: )

1 Like
10

Dang, you are right. I fixed now.

11

I'm curious to know why?

That seems to add so much conceptual baggage to such a simple task.

12

I've noticed before, usually in the context of iterators, that there seems to be a divide between those who find chains-of-combinators-like-expressions intuitive or elegant, and those who find them puzzling or obtuse.

13

I also like the match version best, but I can't resist offering another approach:

values[..i].last().copied().unwrap_or(0) // Look, ma, no subtraction!
10 Likes
14

Quite so.

As it happens your example is pretty clear.

But still it requires comprehension of:

checked_sub
map
closure
unwrapped_or

Where as that simple match expression only requires, well "match".

Perhaps "obtuse" is putting it too strongly but I think that kind statistic is grounds for "overloaded with conceptual baggage"

Still, I have seen such chains that do succinctly and clearly express intension, rather than a mess of loops and and array indexing, for example.

1 Like
15

A local wrapper type machinery.

let prev = {
    struct ZeroCont<'a>(&'a [i32]);
    struct Val(usize);
    struct Sub(usize, usize);
    
    impl<'a> std::ops::Sub<usize> for Val {
        type Output = Sub;
        fn sub(self, rhs: usize) -> Sub {Sub(self.0, rhs)}
    }
    
    impl<'a> std::ops::Index<Sub> for ZeroCont<'a> {
        type Output = i32;
        fn index(&self, i: Sub) -> &i32 {
            if i.0 == 0 {&0} else {&self.0[i.0 - i.1]}
        }
    }

    ZeroCont(&values)[Val(i) - 1]
};
4 Likes
16

Awesome!

17

I think you might be running it in a strange environment, I know weird things have been known to happen in webassembly when trying to handle overflows. I can assure you that it definitely does work. playground

1 Like
18

That's odd. It does indeed work. I must have had some finger trouble somewhere.

19

I think my favourite solution is @drewkett's, (matching on checked sub) because it uses easy to read constructs, isn't unnecessarily verbose, and avoids operations that can panic (except if the index is too big, but that's a whole different thing). @Zicog's is also pretty good (matching on the index), but I agree with @Riateche about how it's nice to put the checking and subtraction in the same operation, so for that reason it feels marginally less safe. (what if for some strange reason I needed to change it from -1 to -2?)

Rust as a language seems to prefer chains of methods to imparative code, which has its pros and cons. In any other language, my second option would seem pretty ridiculous, but I felt like it fits well with that paradigm so I had to try it. Thanks to @juggle-tux for suggesting map_or, I'd never heard of it before and it's a good improvement to what I came up with.

It looks like this solution is the best in terms of least complicated abstractions:

and this one is best if you like the method chaining pattern:

1 Like
20

In a final, desperate, attempt to banish all confusing, abstract, verbose, high level conceptual baggage of combintor chains, match clauses, and even if, I had a shot at tackling this problem the good old C and assembler programmer "branchless" way.

After all we all know the new fangled high level constructs introduce bloat to the executable and branching with if hinders performance with unnecessary branch instructions and the consequent flushing of CPU pipelines.

This then is the ultimate, HLL concept free, clearest way to get the job done:

let prev = (0 - ((i >= 1) as u32)) & values[(0 - ((i > 1) as usize)) & (i - 1)]

Please take a moment to marvel at it's simplicity. It would be simpler if Rust would let me do unary minus and didn't insist on those redundant as. In the absence of any branches this has to be a performance demon.

Except.... This actually generates more instructions than most other solutions here. Especially on the "happy path" where the given index is greater than zero.

Joking, only joking.. honest :slight_smile:

1 Like