Design help - Ownership / Rc / Box / RefCell guidance

edit Made post clearer, improved examples and question.

I'm currently learning Rust and have come across a scenario where I have an object who now needs shared mutable access to x other objects (via trait), and the main function also needs access to those same objects. This issue my be caused by simply bad design / thinking about how to structure the code incorrectly as I'm coming from C++.

Below is a contrived example, but lets imagine I had this code (May have errors):

/// Unique cat object
struct Cat;
impl Cat {
    pub fn cat_specific_fn(&mut self) { ... }
}

/// Unique dog object
struct Dog;
impl Dog {
    pub fn dog_specific_fn(&mut self) { ... }
}

fn main()
{
    let mut x = Cat{};
    let mut y = Dog{};
    
    // Local mutable access here
    x.cat_specific_fn(); // mutates internal state
    y.dog_specific_fn();
}

Now lets assume that this code only contained cats and dogs for some time until you have a requirement to add a "trainer" to train the cats and dogs:

/// Cat and Dog can implement the Animal trait
trait Animal {
    fn train(&mut self);
}
impl Animal for Cat {
    fn train(&mut self) { ... }
}
impl Animal for Dog {
    fn train(&mut self) { ... }
}

/// Trainer whom trains the generic animals via a trait
struct Trainer
{
    animals: Vec<&mut dyn Animal>,
}
impl Trainer
{
    pub fn keep(&mut self, &dyn Animal) { self.animals.push(animal) }
    pub fn train_all_animals(&mut self) { for animals in self.animals { animal.train(); } }
}


fn main()
{
    let mut x = Cat{};
    let mut y = Dog{};
    let mut z = Trainer{};
    
    // Trainer needs mutable access to train animals (uses generic Animal trait)
    z.keep(&mut x); // Saves animals internally for later (there could be 10-20 animals)
    z.keep(&mut y);
    z.train_all_animals(); // Trains x and y
    
    // Local mutable access here
    x.cat_specific_fn();
    y.dog_specific_fn();
    
    // Trainer may train again here 
    z.train_all_animals(); // Borrow Error
}

As you can tell, the above code will cause a compile time error due to attempting to mutable borrow the animals multiple times. The obvious fix is to wrap the original animal types in a Rc<Refcell<X>> type and borrow that out to the classes who need shared ownership, like such:

let x = Rc::new(RefCell::new(Cat{}));
let y = Rc::new(RefCell::new(Dog{}));
let z = Trainer{};

// Trainer needs mutable access to train animals (uses generic Animal trait)
z.keep(&x.clone()); // Saves animals internally for later
z.keep(&y.clone());
z.train_all_animals(); // Trains x and y using interior mutability

// Local mutable access here
x.borrow_mut().cat_specific_fn();
y.borrow_mut().dog_specific_fn();

// Trainer may train again here 
z.train_all_animals(); // Works

And this highlights my question. A requirement (new Trainer object) has forced me to wrap and or modify existing code to facilitate the sharing of resources for the trainer. The original Cat and Dog objects are now useless by themselves as they will only ever be created within a Rc<RefCell<X>> wrapper. This issue could be further exacerbated if exposed via an API.

Should the shared state (or Rc / Box / RefCell / Mutex / etc, etc) of the animal be defined as an impl within the animal?? This will hide the Rc<RefCell> Leakage while keeping the interface. Example:

struct CatState;
impl CatState {
    pub fn do_training(&mut self) { ... }
    pub fn do_something(&mut self) { ... }
}
struct Cat {
    impl: Rc<RefCell<CatState>>
}
impl Animal for Cat {
    fn train(&self) { self.impl.borrow_mut().do_training(); }
}
impl Cat {
    pub cat_specific_fn(&self) { self.impl.borrow_mut().do_something(); }
}
// <DO SAME FOR DOG>

fn main()
{
    let x = Cat{}; // Now has interior Mutability
    let y = Dog{}; // Same here
    let z = Trainer{};
    
    // Trainer simply needs access to train animals (uses generic Animal trait)
    z.keep(&x);
    z.keep(&y);
    z.train_all_animals(); // Trains x and y <OK>
    
    // Local (interior) mutable access here <OK>
    x.cat_specific_fn();
    y.dog_specific_fn();
    
    // Trainer may train again here  <OK>
    z.train_all_animals();
}

My concern with such an approach is now the Cat and Dog can be shared around by a non mutable borrow, which conceals the fact that its internally mutable. Is the impl / shared state with Trainer simply bad design in Rust? What is the idiomatic rust way to deal with this - simply expose the Rc<RefCell> type, Modify Cat/Dog to have a private impl or restructure the code to avoid this issue all together?

Key considerations are that the Trainer might be on another thread.
Examples would be greatly appreciated.

I think it would help to describe the actual problem you have at hand rather than this example. You'd get better advice that way.

1 Like

One idiomatic way to restructure the code, given the example, would be to not store references to the animals in the Trainer object at all. Instead, it could borrow the animals only when it needs to train them:

pub struct Trainer {}

impl Trainer {
    pub fn train_animals(&mut self, animals: &mut [&mut dyn Animal]) {
        for animal in animals {
            animal.train();
        }
    }
}

fn main() {
    let mut x = Cat {};
    let mut y = Dog {};
    let mut z = Trainer {};

    z.train_animals(&mut [&mut x, &mut y]);
    x.pat();
    y.pat();
    z.train_animals(&mut [&mut x, &mut y]);
}

Alternatively, the Trainer could own the animals, and hand out references:

#[derive(Default)]
pub struct Trainer {
    animals: Vec<Box<dyn Animal>>,
}

impl Trainer {
    pub fn new() -> Trainer {
        Default::default()
    }

    pub fn keep(&mut self, animal: impl Animal + 'static) {
        self.animals.push(Box::new(animal));
    }

    pub fn animals(&self) -> &[Box<dyn Animal>] {
        &self.animals
    }

    pub fn animals_mut(&mut self) -> &mut [Box<dyn Animal>] {
        &mut self.animals
    }

    pub fn into_animals(self) -> Vec<Box<dyn Animal>> {
        self.animals
    }

    pub fn train_all_animals(&mut self) {
        for animal in &mut self.animals {
            animal.train();
        }
    }
}

fn main() {
    let x = Cat {};
    let y = Dog {};
    let mut z = Trainer::new();

    z.keep(x);
    z.keep(y);
    z.train_all_animals();
    z.animals_mut()[0].pat();
    z.animals_mut()[1].pat();
    z.train_all_animals();
}
2 Likes

Apologies, I wish I could but Its code at work.
@RedDocMD I have updated post to hopefully be more clear about what I'm doing :slight_smile:

Thanks for the input. The first example is inline with my general thinking, but I feel it may get a tad messy passing in 10+ animals via slice for each invocation. Perhaps there is another way to do so? PS I edited my question to clear some things up.