Seeking advice on architecture coming from a JavaScript background

Hello fellow Rust experts,

I'm seeking some advice for the following code snippet.
As a new years resolution, I challenged myself to finally start getting my hands dirty in Rust and stop just reading about it. I think I got the basics down like working with the borrow checker etc. but the more I work on the following code, the more I have the feeling that I'm doing something which is really against Rust's best practices & design principle. The code just feels wrong.

What I want to achieve:
Creating a rust library that communicates with a Socket.IO server using the rust_socketio crate.
I want to be able to listen for the save event and emit events (the last one is the easy part).

As my professional background is in web technologies (JavaScript, TypeScript) my first idea was to recreate something like "addEventListener". E.g: I have a Communicator which holds a vector of closures (event callbacks) that can be registered. As soon as the save event is triggered by the rust_socketio crate I would then loop through the vector and execute every registered closure. See save_callbacks.

Due to this idea, it is not possible to set the client directly in Communicator::new() as I somehow need a reference to the instance (e.g: self.save_callbacks to iterate over them in the closure).

Therefore I tried to introduce a get_client methods which dynamically creates a SocketIo Client if it doesn't exist. Due to this I now needed to make self mutable (obviously) but also the consumer needs to make the variable mutable (see src/main.rs). It makes total sense but it somehow feels so wrong and I think the approach I'm trying here is not the way to go. Therefore I'm seeking some advice from you on how to improve this.

Remarks:
The code compiles as it is. If you uncomment the callback it fails due to lifetime issues. That's a problem for another day :sweat_smile: I left it in the snippet so that it's clear what my final goal is.

Thanks for the help!

Code

src/lib.rs

use rust_socketio::{Client, ClientBuilder, Payload};

pub struct Communicator {
    client: Option<Client>,
    save_callbacks: Vec<Option<fn(payload: Payload, client: Client) -> usize>>
}

impl Communicator {
    pub fn new() -> Communicator {
        Communicator {
            client: None,
            save_callbacks: vec!(None)
        }
    }

    fn get_client(&mut self) -> &Client {
        match self.client {
            Some(ref client) => client,
            None => {
                // let callback = |payload: Payload, client: Client| {
                //     for callback in self.save_callbacks.iter() {
                //         if let Some(c) = callback {
                //             c(payload.clone(), client.clone());
                //         }
                //     }
                // };
                let client = ClientBuilder::new("http://localhost:3000")
                    .on("error", |err, _| eprintln!("Error: {:#?}", err))
                    // .on("save", callback)
                    .connect()
                    .expect("Connecton failed");
                self.client = Some(client.clone());
                self.client.as_ref().unwrap()
            }
        }
    }
}

src/bin.rs

use sts;

fn main() {
    let mut communicator = sts::Communicator::new();
    //...
}

The on method requires the passed callback to be Send and Sync. So if you want to be able to add listeners after the client is created, you need some way to make things thread safe. Here's an example version of your code using Arc<Mutex<T>>, and with a push_callback method added, to demonstrate that works:

src/lib.rs

use std::sync::{Arc, Mutex};
use rust_socketio::{Client, ClientBuilder, Payload};

type SaveCallback = Option<fn(payload: Payload, client: Client) -> usize>;

type SaveCallbacks = Vec<SaveCallback>;

pub struct Communicator {
    client: Client,
    save_callbacks: Arc<Mutex<SaveCallbacks>>,
}

impl Communicator {
    pub fn new() -> Communicator {
        let save_callbacks: Arc<Mutex<SaveCallbacks>> = Arc::new(Mutex::new(vec![]));

        let save_callbacks_clone: Arc<Mutex<SaveCallbacks>> = save_callbacks.clone();

        let callback = move |payload: Payload, client: Client| {
            let callbacks = save_callbacks_clone.lock().expect("Lock was poisoned");

            for callback in callbacks.iter() {
                if let Some(c) = callback {
                    c(payload.clone(), client.clone());
                }
            }
        };

        let client = ClientBuilder::new("http://localhost:3000")
            .on("error", |err, _| eprintln!("Error: {:#?}", err))
            .on("save", callback)
            .connect()
            .expect("Connecton failed");

        Communicator {
            client,
            save_callbacks,
        }
    }

    pub fn get_client(&self) -> &Client {
        &self.client
    }

    pub fn push_callback(&mut self, callback: SaveCallback) {
        self.save_callbacks.lock().expect("Lock was poisoned").push(callback)
    }
}

src/bin.rs

use sts;

fn main() {
    let mut communicator = sts::Communicator::new();

    let client: &rust_socketio::Client = communicator.get_client();

    communicator.push_callback(None);
}

While we're doing code review, it's unclear to me why the passed callbacks are Options. If there's no function in some case, why put it in the Vec at all? If you don't have a good reason for them to be Options, I suggest you just make them plain function pointers.

1 Like

Thank you so much for your thoroughly detailed answer! It helped me a lot. This was exactly what I was looking for and introduced me to something completely new to me: Arc & Mutex!

I thought there was no way of "getting around" the fact of "only one owner for a piece of data" in Rust and therefore struggled to implement the ClientBuilder directly in ::new(). Now after reading almost the whole day about Rc, Arc, RefCell, Mutex, etc. it makes so much sense to do it your way. I previously didn't know I could have multiple pointers to a shared piece of data that is thread-safe.

In the end, I almost used your code 1 to 1. I just used RwLock instead of Mutex and obviously removed the Option around the save callback. Don't know how I came up with that :sweat_smile:

Again, thank you so much for taking your time and helping a Rust newbie!

1 Like

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.