Why while let comes into an infinite loop?

help
1

here's a find_smallest function, but when i use while let, it whould never return. once i use for loop,it will works well.

pub fn find_smallest(v: &[i32]) -> Option<usize> {
    if v.is_empty() {
        return None;
    }
    let mut smallest = &v[0];
    let mut ans = None;
    //this will never return
    // while let Some((index, current)) = v.iter().enumerate().next() {
    //     if current < smallest {
    //         smallest = current;
    //         ans = Some(index);
    //     }
    // }
    //but this works well
    for (i, n) in v.iter().enumerate() {
        if n < smallest {
            smallest = n;
            ans = Some(i)
        }
    }
    ans
}
2 Likes
2 
while let Some((index, current)) = v.iter().enumerate().next() {
     if current < smallest {
         smallest = current;
         ans = Some(index);
     }
}

v.iter().enumerate() would create a new iterator. So when reach this code, you create a new iterator, and the condition never change.

and for loop is a syntactic sugar in rust:

5 Likes
3

You're creating a new iterator at the beginning of every loop, which always gives you the first item in the slice. For it to work, you need to create an iterator once, outside of the loop, like this:

let mut iterator = v.iter().enumerate()

while let Some((index, current)) = iterator.next() {
    if current < smallest {
        smallest = current;
        ans = Some(index);
    }
}
10 Likes
4

Just to be sure, you don't need that much code for that

5
1 Like
6

OP is looking for the index of the smallest item, you are returning the item itself. Same for @Schard's proposed solution.

You'll need something slightly longer to match what OP's function does Rust Playground

2 Likes
7
1 Like
8

This is doing two passes through the array, which is slower.

1 Like
9

Your solution does an unnecessary extra comparison when elements are equal. Here is a better and shorter version:

pub fn find_smallest(v: &[i32]) -> Option<usize> {
    v.iter()
        .enumerate()
        .min_by_key(|x| x.1)
        .map(|x| x.0)
}
10 Likes
10

Nitpicking:

While I’m not sure if this was intended by the original author, all the posted alternative code examples do not match the original version:

In the original code, if the first element in the slice is the smallest, None is returned.

However, I believe this is likely an error.

10 Likes
11

But of course we can reproduce this behaviour :stuck_out_tongue_winking_eye:

    v.iter()
        .enumerate()
        .min_by_key(|(_idx, value)| *value)
        .map(|(idx, _value)| idx)
        .map_or(None, |idx| if idx == 0 {
            None
        } else {
            Some(idx)
        })
2 Likes
12

FYI the map_or can be replaced with a much shorter .filter(|idx| idx != 0)

4 Likes
13

FYI I filed an issue with clippy to lint this: new lint: option manual filter `Option::and_then(|x| { if ... then {None} else {Some(x) })` · Issue #14440 · rust-lang/rust-clippy · GitHub