Code review: design choice: Clone or Rc in container?

I'm new to Rust and I'm trying to create a container like Container<T>. Due to implementation details I need to clone values of type T during normal operation. Should I change all uses of T into Rc<T> instead, and not require T implement Clone?

For example, have a look at this persistent (in the 'old versions preserved' sense):

impl<T: Clone> Queue<T> {
    pub fn new() -> Queue<T> {
        // ...
    }

    pub fn pop_front(&self) -> Option<(Queue<T>, T)> {
        // ...
    }

    pub fn push_back(&self, v: T) -> Queue<T> {
        // ...
    }
}

Would it be better if I didn't require T: Clone, change pop_front to return Option<(Queue<T>, Rc<T>)>, and change the implementation accordingly?

The code can be found here: https://github.com/dramforever/rust-rtqueue/blob/master/src/lib.rs. Any other code review suggestions are also welcome.

It seems to me that the implementation with you have is more general, since the user can explicitly select a Queue<Rc<T>>. Its clone() will then do what you would do anyway. The downside is that if the Rc instantiation is used overwhelmingly, it is a bit less convenient for the client code (having to write Queue<Rc<T>> and create the Rc themselves in push.)

Not only is your initial design more general, but it is superior in the case where T is a type like i32 or &T. I wouldn't make Rc part of the type unless you plan to do something that requires them (like doing something clever with Weak pointers).

I would make the documentation for affected methods (or the type if it affects most methods) explicitly call out that it produces clones of T for such and such reasons. (Technically speaking, the documentation already implies this when it shows the T: Clone bounds, but that's easy for the reader to overlook.)

Thank you both for the advice. I think you're right and requiring Clone and documenting it is way to go.

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.