Do I need interior mutability here?

help
#1

So I have a relatively contrived toy example.

I've fiddled with this such that my ego has sufficiently depleted to the point where I'm asking myself the deep questions like, "is this even possible without interior mutability?" IM lives in a spot in Rust I haven't had to play around with very much. Would love if anyone would recommend a blog post on the topic :slight_smile:

Back to the problem at hand, I have a pair of toy tuple structs, one of which is just a a vector with ownership of the other.

Should I be modifying my wrapping to look like
struct OwnsOffer(Vec<RefCell<Offer>>);,
or is there something dumb I'm overlooking?

struct Offer(u64, bool);
impl PartialEq for Offer {
    fn eq(&self, other: &Offer) -> bool {
        self.0 == other.0
    }
}
// these trait bounds are satisfied just so I can use max on an iterator, SKIP TO ...
impl PartialOrd for Offer {
    fn partial_cmp(&self, other: &Offer) -> Option<std::cmp::Ordering> {
        if self.0 > other.0 {
            Some(std::cmp::Ordering::Greater)
        } else if self.0 == other.0 {
            Some(std::cmp::Ordering::Equal)
        } else {
            Some(std::cmp::Ordering::Less)
        }
    }
}
impl Eq for Offer {}

impl Ord for Offer {
    fn cmp(&self, other: &Offer) -> std::cmp::Ordering {
        self.partial_cmp(other).unwrap()
    }
}
struct OwnsOffer(Vec<Offer>);

// SKIP TO HERE
impl OwnsOffer {
    pub fn push_new_offer(&mut self, offer: Offer) {
        self.0.push(offer);
    }
    pub fn get_highest_offer(&self) -> Option<&Offer> {
        self.0.iter().max() 
    }
    pub fn accept_highest_offer(&mut self) {
        let offer = self.get_highest_offer();
        match offer {
            None => panic!("accept highest active offer called on no active offers"), // I'm aware this is silly
            Some(offer) => offer.1 = true,
        }
    }
}
#2

You can just add a _mut version of get_highest_offer if you need mutable access to the highest offer it found. The implementation is very straightforward, just adding the word mut in a few places:

pub fn get_highest_offer_mut(&mut self) -> Option<&mut Offer> {
    self.0.iter_mut().max()
}
pub fn accept_highest_offer(&mut self) {
    let offer = self.get_highest_offer_mut();
    match offer {
        None => panic!("accept highest active offer called on no active offers"), // I'm aware this is silly
        Some(offer) => offer.1 = true,
    }
}
1 Like
#3

Also note that you can simplify your Ord/PartialOrd implementation:

impl Ord for Offer {
    fn cmp(&self, other: &Self) -> std::cmp::Ordering {
        self.0.cmp(&other.0)
    }
}
impl PartialOrd for Offer {
    fn partial_cmp(&self, other: &Self) -> Option<std::cmp::Ordering> {
        Some(self.cmp(other))
    }
}
1 Like
#4

Thanks for the killer tips @steffahn ! One more bit, if you're still around. I'm struggling to understand what's going on here, that I can't call a helper function transfer on the highest_offer you just helped me figure out how to get.

struct Offer(u64, bool);
impl PartialEq for Offer {
  fn eq(&self, other: &Offer) -> bool {
    self.0 == other.0
  }
}

impl Ord for Offer {
  fn cmp(&self, other: &Offer) -> std::cmp::Ordering {
    self.0.cmp(&other.0)
  }
}

impl PartialOrd for Offer {
  fn partial_cmp(&self, other: &Offer) -> Option<std::cmp::Ordering> {
    Some(self.cmp(other))
  }
}

impl Eq for Offer {}

struct OwnsOffer(Vec<Offer>);

impl OwnsOffer {
    pub fn get_highest_active_offer_mut(&mut self) -> Option<&mut Offer> {
        self.0.iter_mut().max()
    }

    pub fn accept_highest_active_offer(&mut self) {
        let _ = &self
            .get_highest_active_offer_mut()
            .and_then(|a| Some(self.transfer(a)));
    }
    pub fn transfer(&mut self, offer: &mut Offer) {
        //offer.accept();
        //self.owner = offer.from.clone();
    }
}
#5

Though the equivalent seems to work:

  pub fn accept_highest_active_offer(&mut self) {
    let offer = self.get_highest_active_offer_mut();
    match offer {
      None => panic!("accept highest active offer called on no active offers"),
      Some(offer) => {
        offer.accept(); // toggles offer bool
        self.1 = offer.from.clone(); // in my actual struct, there's an additional string field 

        //        &mut self.transfer(offer);
      }
    }
  }
#6 
pub fn transfer(&mut self, offer: &mut Offer) {
        //offer.accept();
        //self.owner = offer.from.clone();
    }

This won’t work, since both the reference to self as the offer reference promise to be uniquely accessing their respective targets, even though the Offer is (at least in your call) contained in self.

You can either not create any method abstraction around actions that need both: first mutable access to a concrete Offer and afterwards to the whole OwnsOffer it’s contained in, or you could refer to the Offer by an index. (Or, if you want to abstract something like that, split it into two parts accordingly.)

Without a transfer method:

impl Offer {
    fn accept(&mut self) {
        self.1 = true;
    }
}

impl OwnsOffer {
    pub fn get_highest_active_offer_mut(&mut self) -> Option<&mut Offer> {
        self.0.iter_mut().max()
    }

    pub fn accept_highest_active_offer(&mut self) {
        let _ = &self
            .get_highest_active_offer_mut()
            .map(|a| {
                a.accept();
                a
            })
            .cloned() // compiler can infer that no reference
                      // to a is used anymore after this point
            .map(|a| self.set_owner(a)); // hence self can be used again here
    }
    pub fn set_owner(&mut self, offer: Offer) {
        //self.owner = offer.from.clone();
    }
}

With indices:

impl OwnsOffer {
    pub fn find_highest_active_offer(&mut self) -> Option<usize> {
        self.0
            .iter_mut()
            .enumerate()
            .max_by(|(_, x1), (_, x2)| x1.cmp(x2))
            .map(|(ix, _)| ix)
    }
    pub fn accept_highest_active_offer(&mut self) {
        let _ = &self
            .find_highest_active_offer()
            .and_then(|i| Some(self.transfer(i)));
    }
    pub fn transfer(&mut self, i: usize) {
        let offer = &mut self.0[i];
        offer.accept();
        let offer = offer.clone();
        self.set_owner(offer);
    }
    pub fn set_owner(&mut self, offer: Offer) {
        //self.owner = offer.from.clone();
    }
}
1 Like
#7

As you might have noticed, a match statement (like you used here) makes this a lot more comfortable than the chained .map()s that I used.

1 Like
#8

Ah, so a little hidden gem of information here is that the compiler uses the clone call to infer mutable reference to the Offer ends there. That explains why the commented code at the bottom of this post would not compile, but can easily be fixed. Cool, super useful. I do think the match statement looks a bit clearer here.

  pub fn accept_highest_active_offer_maps(&mut self) {
    let offer = self.get_highest_active_offer_mut();
    let _ = &self
      .get_highest_active_offer_mut()
      .map(|offer| {
        offer.accept();
        offer.from.clone()
      })
      .map(|a| self.set_owner(a));
  }
  pub fn set_owner(&mut self, offer: String) {
    self.owner = offer.clone();
  }

  pub fn accept_highest_active_offer_match(&mut self) {
    match self.get_highest_active_offer_mut() {
      Some(offer) => {
        offer.accept();
        let new_owner = offer.from.clone(); // compiler infers no reference to offer is used beyond clone
        self.set_owner(new_owner); // allowing this
                                   // but not:
                                   //self.set_owner(offer.from.clone()); // ERROR: shared mutability borrows
      }
      None => (),
    }
1 Like