Continuing from: Read/write graph nodes with error handling
I'm trying to make the most user friendly and flexible API I can for my crate about graph data. When you want to read or write to a graph part (node, leaf, other stuff), you must provide some kind of callback so the graph can do some logic before and after.
Option A, the graph part could implement this trait:
pub trait Read {
type Payload;
/// Read the payload of the graph part.
fn read<T, F>(&self, reader: F) -> graph::Result<T>
where
F: FnOnce(&Self::Payload) -> T;
}
User code example:
node.read(|payload| myLocal += payload);
Option B:
pub trait Read {
type Payload;
/// Read the payload of the graph part.
fn read<T>(&self, reader: &dyn Reader<T, &Self::Payload>) -> Result<T>;
}
pub trait Reader<T, P> {
fn reader(&mut self, payload: &P) -> T;
}
User code example:
node.read(&thingThatiImplsReader)
I think this might be about preference but I could be missing some things to consider. My problem with Option A is that it gets messy quick. You can pass in a longer function and do node.reader(myCallback) but myCallback won't have useful context information like some accumulator. I like Option B because the trait object could have extra context info. However, with option A, users can just do node.read(|value| myStruct.doSomething(value)); to have extra context in doSomething. It is not as short as node.read(&myStruct); but at least there is the flexibility to pass an anonymous function. Am I missing some consideration?
Don't force references and especially not trait objects in the signature:
Forcing the caller to pass a reference when it's not actually necessary is pure annoyance.
You can go from a generic to a trait object, but you can't go back.
You don't even make the trait object-safe, as the method already has another type parameter.
Since Rust has closures, you don't need the context information. If it's something that can be extracted from the object via a public API, you should make that the responsibility of the caller (otherwise, how'd you decide where to stop? there's no way you can anticipate all kind of context that callbacks may ever anticipate.)
You can just blanket-impl your custom callback trait for functions if you need to. For example, if the context parameter is actually useful sometimes (and the callback can't obtain it by other means), but it will be ignored most of the time, you can implement the trait for Fn* types by simply ignoring that argument.
Thanks for the detailed help. It sounds like Option A is the way to go based on your feedback. I see your point about not forcing a reference. Are you suggesting that if the user choices to do so, it would be acceptable for them to impl FnOnce for their struct and then they can call node.read(myStruct)? And when I say extra "context" info, I just mean information in myStruct or local variables if a closure is used. The read function does not need to provide anything more but a reference to the payload.
No, that's not possible. Fn* traits are unstable to implement manually; one can only ever obtain Fn-implementing values via built-in language constructs (fn items, fn pointers and closures).
If you want other, explicitly user-defined types to be supplied, then you'll need a custom trait. And you blanket-impl your custom trait for Fn* types so they are usable as a callback out of the box.
No, all in all, it looks like you want neither. You want a combination of the two (do make a custom trait, but lose the trait object and the mandatory reference, and blanket-impl for anything implementing FnOnce).
pub trait Read {
type Payload;
/// Read the payload of the graph part.
fn read<T, R>(&self, reader: R) -> graph::Result<T>
where
R: PayloadReader<T, Self::Payload>;
}
pub trait PayloadReader<T, Payload> {
fn read_payload(self, payload: Payload) -> T;
}
impl<Payload, T, Func: FnOnce(Payload) -> T> PayloadReader<T, Payload> for Func {
fn read_payload(self, payload: Payload) -> T {
self(payload)
}
}
This keeps the shape of Read you had before, allows you to use closures as the type of reader (because of the blanket implementation), but gives you the freedom to implement PayloadReader for a more complicated structure than a closure.
Thank you so much for the code snippets to show what paramagnetic was saying! This is awesome. It's opened my thinking on what I can do for other parts of the project as well.