Newbie question: is there a preference to having a struct with several RefCells or making the outer struct a single RefCell?

I'm writing a graphics app that has state information that needs to be passed through closures. Some of the state is immutable, which isn't a problem. There are 6 or so mutable state variables. I originally wrote it up with each of them being wrapped in a Cell or RefCell, and this works, but makes the code harder to read since even simple assignments need to call .get() or .set().

I then rewrote it using a new struct MutableState that contains all the mutable state variables and wrap that one value in a RefCell. This way a single borrow() at the start of a method makes all the variable accesses look much more natural. It has a cost though: if a lower level method needs mutable access to a state variable the state cannot be borrowed in scope anywhere above in the call stack. This can be handled by making sure references to the state drop out of scope before making calls that might need mutable access.

In general, is one of these two patterns considered better form than the other? Is there a general feeling one is better, or would it just be a case of whichever seems appropriate in the application?

In general, it's considered good style to pass mutable references when you need mutation.

Consequently this:

…should instead be resolved by borrowing mutably and passing down a proper mutable reference to methods that need mutation. This in turn allows the single-borrow approach. I'd be suspicious if I saw 4 or 6 simultaneous .borrow() and .borrow_mut() calls for each field – although it still might be the right solution in some cases. But I'd be inclined to think that there probably is a missing abstraction (i.e., a type encapsulating those 4…6 fields).

1 Like

It might have helped if I had included a concrete example in the question. I think your meaning is that it would be preferable to treat the mutable and immutable elements the same, just go with a plain structure, and pass it down as mutable even when it doesn't need to be, so it only needs to be borrowed once at the head of the chain.

I just tried that - thanks, it seems a better option. It can still be a little tricky - there were a couple places I had to mess around with definitions to get values that didn't reference the struct, but that may just be me learning to think like a Rust programmer. It is as simple as my first attempt and as easy to read as my second. I guess if I were writing a package to be public and wanted to keep some state internal the second pattern would makes sense, but in this case I don't care if all the state is visible. Also, I've been trying to make as little mutable as possible, so making a whole chain of method calls mut &self kind of grates, but I have no concrete reason to feel that way.

Thanks, this is definitely a better way to do it, and I feel has improved my mental vision of Rust.



#[derive(Debug, Clone)]
pub struct Board {
    // immutable
    num:           usize,
    width:         i32,
    height:        i32,
    window:        gtk::Window,
    grid:          gtk::Grid,
    command_hash:  HashMap<String, Command>,
    // mutable (maybe put these in a STATE struct?)
    p_x:           Cell<i32>,
    p_y:           Cell<i32>,
    p_orientation: Cell<Orientation>,
    p_piece:       Cell<&'static Piece>,
    p_next_piece:  Cell<&'static Piece>,
    p_bitmap:      RefCell<Vec<u32>>,
}

or

#[derive(Debug, Clone)]
pub struct Board {
    // immutable
    num:           usize,
    width:         i32,
    height:        i32,
    window:        gtk::Window,
    grid:          gtk::Grid,
    command_hash:  HashMap<String, Command>,

    p_state: RefCell<State>,
}
#[derive(Debug, Clone)]
struct State {
    x: i32,
    y: i32,
    orientation: Orientation,
    piece: &'static Piece,
    next_piece: &'static Piece,
    bitmap: Vec<u32>,
}

3rd way:

#[derive(Debug, Clone)]
pub struct Board {
    // immutable
    num:           usize,
    width:         i32,
    height:        i32,
    window:        gtk::Window,
    grid:          gtk::Grid,
    command_hash:  HashMap<String, Command>,
    // mutable (maybe put these in a STATE struct?)
    x:           i32,
    y:           i32,
    orientation: Orientation,
    piece:       &'static Piece,
    next_piece:  &'static Piece,
    bitmap:      Vec<u32>,
}

(Abstract answer; other answers are probably better for your specific use.)

You can think of RefCell as a single-threaded mutex, if that helps. Then you can re-use your intuition about how granular you want to make mutexes: big enough to cover everything that needs to be used together, for example.

This topic was automatically closed 90 days after the last reply. We invite you to open a new topic if you have further questions or comments.