austinjones / postage-rs

The feature-rich, portable async channel library
MIT License
252 stars 9 forks source link

Idea: channel should be a type, not a function. #63

Open blueforesticarus opened 1 year ago

blueforesticarus commented 1 year ago

I notice almost every time I use postage I end up having to specify the channel type many times. Some places this can be reduced with impl postage::Sink<...> impl postage::Stream, but always.

I think that channel should be a type defined per channel type like Sender and Sink. With a corresponding trait.

// in mpsc
struct Channel(usize)

impl crate::Channel for Channel {
    type Sender = Sender;
    type Receiver = Receiver;

   fn create() -> (Sender, Receiver) {
      // behavior of current create function.
       ...
    }
}

This enables (imo) cleaner code when using channels. See this example from some code I'm writing using the matrix-sdk.

Currently:

let (tx, message_rx) = postage::mpsc::channel::<types::Message>(100);
c.client.add_event_handler_context(tx);
c.client.add_event_handler(
    |event: OriginalSyncRoomMessageEvent,
     room: Room,
     channel: Ctx<postage::mpsc::Sender<types::Message>>| {
        ...
    },
);

Improved:


type MsgChannel = postage::mpsc::Channel(100);
let (tx, message_rx) = MsgChannel::create();
c.client.add_event_handler_context(tx);
c.client.add_event_handler(
    |event: OriginalSyncRoomMessageEvent,
     room: Room,
     channel: Ctx<MsgChannel::Sender>| {
        Self::on_room_message(event, room, channel.0)
    },
);

This makes refactoring easier if we decide we need to use a different channel type. It also makes the contract explicit. Sometimes it is less important what type of channel is used, and more important that the Sender and Receiver types line up (perhaps 100s of lines away, or in a different file). I have often had to refactor a mpsc into a broadcast channel, and this would help in those cases. There is a similar case for readability.

It also means we may be able to define generic functions over channel types.

blueforesticarus commented 1 year ago

Similarly, it may make sense for Senders and Receivers to have traits to gain access to their opposite type. Though I think there are some issue with that idea

blueforesticarus commented 1 year ago

playing around

// Maybe this should hold the sender and receiver
pub struct Mpsc<T, const S: usize>(std::marker::PhantomData<T>);

pub trait Channel {
    type Sender: postage::sink::Sink;
    type Receiver: postage::stream::Stream;

    fn new() -> (Self::Sender, Self::Receiver);
}

impl<T, const S: usize> Channel for Mpsc<T, S> {
    type Sender = postage::mpsc::Sender<T>;
    type Receiver = postage::mpsc::Receiver<T>;

    fn new() -> (Self::Sender, Self::Receiver) {
        postage::mpsc::channel(S)
    }
}

Unfortunately due to ambiguous associate types I end up having to use the qualified syntax, which is not ideal.

use crate::types::{Message, Reaction};
use crate::utils::channel::{Channel, Mpsc};
//XXX: What is the correct way to define this. I still don't know

pub type MessageChannel = Mpsc<Message, 100>;
pub type ReactChannel = Mpsc<Reaction, 100>;

pub trait ChatService {
    // Note: weirdness with ambiguous type
    fn message_channel(&mut self) -> &mut <MessageChannel as Channel>::Receiver;
    fn react_channel(&mut self) -> &mut ReactChannel::Receiver;

    //fn rescan_since();
}
error[E0223]: ambiguous associated type
  --> src/traits.rs:11:41
   |
11 |     fn react_channel(&mut self) -> &mut ReactChannel::Receiver;
   |                                         ^^^^^^^^^^^^^^^^^^^^^^ help: use fully-qualified syntax: `<utils::channel::Mpsc<types::Reaction, 100> as Trait>::Receiver`

This strikes me as odd and may be related to https://github.com/rust-lang/rust/issues/38078