Is the following a code smell?

1

a, b, c, d, e, f are fns that take more arguments than just &self; I'm just too lazy to list them

pub trait Foo_Mut_T {
    fn a(&mut self);
    fn b(&mut self);
    fn c(&mut self);
    fn d(&mut self);
    fn e(&self);
    fn f(&self);
}

pub trait Foo_T {
    fn a(&self);
    fn b(&self);
    fn c(&self);
    fn d(&self);
    fn e(&self);
    fn f(&self);
}

impl<T: Foo_Mut_T> Foo_T for RefCell<T> {
    fn a(&self) {
        self.borrow_mut().a();
    }

    fn b(&self) {
        self.borrow_mut().b();
    }

    fn c(&self) {
        self.borrow_mut().c();
    }

    fn d(&self) {
        self.borrow_mut().d();
    }

    fn e(&self) {
        self.borrow().e();
    }

    fn f(&self) {
        self.borrow().f();
    }
}

This feels a bit repetitive defining nearly the same trait twice. I don't see a way around this, to go from T -> RefCell<T>.

Do others run into this? (Is this a legit issue?)

Is this a code smell / hint I should be redesigning things ?

1 Like
2

There's currently no way round this, but this SO answer suggests to use the duplicate crate as a workaround.

2 Likes
3

Perhaps. We have no hint of the wider context. I'd say it could be perfectly valid, or a sign that reasonable compromises are not being made. It really depends on the overall objective ( performance-wise ).

What I mean is that coding in Rust is a perpetual sequence of choices and compromises that have to be made, Rust gives you choices to make, you have to make them.

1 Like
4

Imho locking a RefCell inside of function implementations is a bit of a code smell on its own. On one hand, it's not reasonable to expose an inner RefCell all the way to the top datastructures. But on the other, when all you already have is a RefCell, just silently locking it is asking for a runtime panic. That's not much of a concern for your simple one-liner function implementations, but a real issue for more complex code.

In my opinion, it's cleaner, clearer and more composable to just lock the RefCell explicitly where needed, and don't implement Foo_T for it at all. If locking is so pervasive that it's an issue, it may be a sign that you need to restructure your code in some way.

1 Like
5

Another option would be to implement Foo_Mut_T for &'_ RefCell<T>. Passing &mut &RefCell<T> or &&RefCell<T> arguments is a little ugly, but should give you similar semantics to Foo_T without needing a separate trait.

1 Like
6

Do Foo_Mut_T::a and Foo_T::a share a similar name by coincidence, or on purpose? As in, do both of those a functions do the same thing?

7

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.