bretmattingly
bretmattingly

Reputation: 257

Iterate and take from vector of Options

Every time I think I've got idiomatic Rust a little bit figured out, it defeats me again. In this case, I've got a struct State which contains, among other things, a Vec<Option<Dude>> called ods, where Dude is a struct that looks like this:

pub struct Dude {
    pub capacity: i32,
    pub status: DudeStatus,
}

What I would LIKE to do is define a function/method on State that iterates through ods. If a Dude is present in the given index (that is, if ods[i] == Some(Dude)), decrement his capacity by one, and if that causes capacity==0, remove the Dude from ods. Unfortunately, I seem to be running into type inference and/or ownership issues. Here's my attempt:

fn tick(&mut self) {
    for d in &self.ods {
        match d {
            Some(du) => {
                du.capacity -= 1;
                if du.capacity == 0 {
                    d.take();
                }
            }
            None => {}
        };
    }
}

However, this gives 3 compilation errors:

src/state.rs:40:18: 40:26 error: mismatched types:
 expected `&std::option::Option<dude::Dude>`,
 found `std::option::Option<_>`
 (expected &-ptr,
 found enum `std::option::Option`) [E0308]
src/state.rs:40                  Some(du) => {
                                 ^~~~~~~~

src/state.rs:46:18: 46:22 error: mismatched types:
 expected `&std::option::Option<dude::Dude>`,
 found `std::option::Option<_>`
 (expected &-ptr,
 found enum `std::option::Option`) [E0308]
src/state.rs:46                  None         => {}

src/state.rs:41:22: 41:32 error: the type of this value must be known in this context
src/state.rs:41                      du.capacity -=1;

The third error is easy enough to understand conceptually, but I'm not sure where I should be annotating the type. Type ascription is currently an experimental feature so I can't use match d: Option<Dude> which, to me, is the most intuitive. Additionally the other two errors suggest I have a borrow/reference error. What am I doing wrong?

Upvotes: 2

Views: 3723

Answers (2)

kennytm
kennytm

Reputation: 523304

fn tick(&mut self) {
    for d in &self.ods {

This will give immutable ds. Since you want to modify d you should iterate on &mut self.ods instead.

        match d {
            Some(du) => {

Note that d is a reference. You should deference it match *d. And you want to capture du by mutable reference (Some(ref mut du) =>) so your changes will be propagated back to the original storage.

As the None arm is doing nothing you could use if let instead of match to save an indentation level.

                if du.capacity == 0 {
                    d.take();
                }

After we fixed the above two issues, the borrow checker will complain here. This is expected, as the borrow of du from d is still valid, but you now try to destroy d. We need to call .take() outside of the match. Personally I would add a flag to indicate if we need to nullify d. Note that since you are not using the result, simply calling *d = None is enough.


Final result:

fn tick(&mut self) {
    for d in &mut self.ods {
        let mut should_nullify = false;
        if let Some(ref mut du) = *d {
            du.capacity -= 1;
            if du.capacity == 0 {
                should_nullify = true;
            }
        }
        if should_nullify {
            *d = None;
        }
    }
}

Upvotes: 4

Shepmaster
Shepmaster

Reputation: 430791

Here's the code I used to fill in the aspects you didn't provide:

struct Dude {
    pub capacity: i32,
}

struct State {
    ods: Vec<Option<Dude>>,
}

impl State {
    fn tick(&mut self) {
        for d in &self.ods {
            match d {
                Some(du) => {
                    du.capacity -= 1;
                    if du.capacity == 0 {
                        d.take();
                    }
                }
                None => {}
            };
        }
    }
}

fn main() {}

Checking out the first error:

error: mismatched types:
 expected `&core::option::Option<Dude>`,
    found `core::option::Option<_>`
(expected &-ptr,
    found enum `core::option::Option`) [E0308]

                 Some(du) => {
                 ^~~~~~~~

The match statement is looking for the match arm to be a &Option<Dude, but the pattern represents an Option<_> ("option of an as-yet-undetermined type"). If you change it to &Some or (more idiomatically) match on *d, that error will go away.

Then you run into an issue where you are iterating immutably, so that needs to be fixed (for d in &mut self.ods and Some(mut du)).

Then you are trying to move out of the vector into the Some pattern, so you need Some(ref mut du).

Then you run into the real problem: you are trying to mutate the Option while referencing its internals. This is invalid and would lead to memory safety issues!

Instead, split up the concept of decrementing the count and removing the values:

fn tick(&mut self) {
    for d in &mut self.ods {
        let remove = match *d {
            Some(ref mut du) => {
                du.capacity -= 1;
                du.capacity != 0
            }
            None => false,
        };

        if remove {
            d.take();
        }
    }
}

If you wanted to remove from the Vec, you could also use retain:

fn tick(&mut self) {
    for d in &mut self.ods {
        if let Some(ref mut du) = *d {
            du.capacity -= 1;
        }
    }

    self.ods.retain(|d| {
        d.as_ref().map_or(false, |du| {
            du.capacity != 0
        })
    })
}

Upvotes: 5

Related Questions