Preventing code duplication when implementing a trait

[guerlando]

I have a trait called Stoppable that makes it possible to create struct instances that have their own thread and that stop this thread when they are dropped.

I have multiple structs that need to implement Stoppable. For example: RtspClient, Decoder, Renderer, ImageProcessor.

The problem is that to implement Stoppable for each of them, I need to add

        should_continue: Arc<AtomicBool>,
        run_thread: Option<JoinHandle<()>>,

to all of them.

Also, I tried doing this:

        fn should_continue(&self) -> bool {
            self.should_continue.load(Ordering::Relaxed)
        }

but I ended up having to ditch it and use this function inside the while like this:

while should_continue_.load(Ordering::Relaxed)  {

because of the closure (otherwise it'd move the borrowed &self).

Take a look:

    use super::decoded_packet::DecodedPacket;
    use super::decoder::{Codec, Decoder};
    use super::defaults;
    use super::encoded_packet::EncodedPacket;
    use super::stoppable::Stoppable;
    use std::sync::atomic::{AtomicBool, Ordering};
    use std::sync::Arc;
    use std::thread::{sleep, spawn, JoinHandle};

    pub struct DummyDecoder {
        pub on_consume: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
        pub on_produce: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>,
        should_continue: Arc<AtomicBool>,
        run_thread: Option<JoinHandle<()>>,
    }

    impl DummyDecoder {
        pub fn new(
            on_consume: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
            on_produce: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>,
        ) -> DummyDecoder {
            DummyDecoder {
                on_consume: on_consume,
                on_produce: on_produce,
                should_continue: Arc::new(AtomicBool::new(true)),
                run_thread: None,
            }
        }
    }

    impl Decoder for DummyDecoder {
        fn set_on_consume(
            &mut self,
            f: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
        ) {
            self.on_consume = f;
        }
        fn set_on_produce(&mut self, f: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>) {
            self.on_produce = f;
        }
        fn codec(&self) -> Codec {
            Codec::H264
        }
    }

    impl Stoppable for DummyDecoder {
        fn should_continue(&self) -> bool {
            self.should_continue.load(Ordering::Relaxed)
        }

        fn run(&mut self) {
            let on_consume_ = self.on_consume.clone();
            let on_produce_ = self.on_produce.clone();
            let should_continue_ = self.should_continue.clone();
            self.run_thread = Some(spawn(move || {
                while should_continue_.load(Ordering::Relaxed) {
                    let encoded_packet = (on_consume_)();
                    match encoded_packet{
                        Some(encoded_packet) => {
                            println!("decoder received packet!")
                        }
                        None => {}
                    }
                    //Simulate transform of encoded_packet in decoded_packet here
                    sleep(defaults::default_timeout_stoppable);
                    let decoded_packet = DecodedPacket { data: Vec::new() };
                    (on_produce_)(decoded_packet);
                }
            }));
        }
        fn stop(&self) {
            self.should_continue.store(false, Ordering::Relaxed);
        }
    }

    impl Drop for DummyDecoder {
        fn drop(&mut self) {
            self.stop();
            self.run_thread.take().unwrap().join();
        }
    }

On C++, should_continue, run_thread and fn should_continue() would be shared in a common base class. I just want to confirm that there's no way to reuse these variables and functions, because I hate copying the same code over and over for the same class.

[droundy]

Sounds like a case where you want a strict rather than a trait. Create a wrapper for a join handle that joins when dropped, and then store this wrapper in each of your Stoppable types.

[trentj]

I haven't understood the point of Stoppable, but I'm wondering if it could be an adapter similar to Peekable for iterators -- so you could have Stoppable<RtspClient>, Stoppable<Decoder>, etc. as well as the non-wrapped versions.

[guerlando]

what is a strict? Can you give me a pseudocode example of your idea? I'm trying to figure out how an object would be able to join the thread if it does not know the thread object

[carey]

It's a typo for struct.

What you might have is a struct Stopper<T: Stoppable>(inner: T) with an impl<T: Stoppable> Deref for Stopper<T> that returns a reference to the inner value, and implement should_continue and run_thread on Stopper.

[guerlando]

and what would be the inner value? My struct DummyDecoder?

Like:

struct Stopper<DummyDecoder> {
         pub on_consume: Arc<dyn Fn() -> Option<EncodedPacket> + Send + Sync + 'static>,
        pub on_produce: Arc<dyn Fn(DecodedPacket) + Send + Sync + 'static>,
        should_continue: Arc<AtomicBool>,
        run_thread: Option<JoinHandle<()>>,
}

?

I'm trying to understand this approach. On C++ I'd do

class DummyDecoder: public Stoppable, public Class1, public Class2, ... {

}

This approach that you told me kinda limits me to inherit from Stoppable only, I could not make this for more classes as Class1, Class2, etc, right? Not that this is a problem, I'm just trying to understand the design.

[carey]

I’d need to think more about what you’re describing in order to mock up a good example, and not make the wrong assumptions.

A design based on multiple inheritance isn’t going to work well in Rust, but I haven’t got enough experience myself to describe the right alternatives.

[droundy]

I'm thinking something more like:

struct StopThread {
     should_continue: Arc<AtomicBool>,
     run_thread: Option<JoinHandle<()>,
}
impl Drop for StopThread { ... }
struct YourData {
  t: StopThread,
  ...
}

So you add whatever constructers and methods you like. Then as a field for a StopThread in any struct that you want to have the feature. I'm not clear on precisely what behavior you want, but what you've described seems like it doesn't require traits at all, and there's no duplicated code.