-
Notifications
You must be signed in to change notification settings - Fork 152
Shared state pattern (spin-off of #427) #430
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
As a user it seems simpler to avoid the |
This is the main point of controversy that I expect for this PR. I think there are fair arguments in both directions. We're not exactly hiding anything about what There's a fair question of how it looks inside a code review where type hints aren't readily available. Would someone reviewing the code at a glance be concerned when they see There are plenty examples of other objects in the standard library and third-party libraries where cloning an object creates a new handle to the same object, e.g. |
Yes, that's true and in Rust the Clone is not automatic so we can guess that if it is available, its ok to use it. |
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
Signed-off-by: Michael X. Grey <[email protected]>
This PR is spun out of #427 and focuses entirely on moving the API structs to a "shared state" pattern, reducing the prevalence of
Arc
in the API without any functional changes.This one is very subjective so I don't mind discarding it, but I felt that the prevalence of
Arc
in the API was adding noise that is seldom relevant to the average user. In this PR, I've refactored the API a little so thatNode
,Subscription
,Publisher
,Client
, andService
each have a respective_State
structure which is public and represents the underlying instance. At the same time,Node
,Subscription
, etc are now type aliases of the formpub type Node = Arc<NodeState>;
pub type Subscription<T> = Arc<SubscriptionState<T>>;
This does not lead to any actual change in the structure or memory management of any of these types; it's a purely superficial renaming. But it reduces how often
Arc
is tossed around in the API and thereby streamlines the 95% (my own estimate) of use cases whereArc
is just an implementation detail. At the same time, it does not interfere with the remaining 5% of use cases where theArc
is relevant, e.g. if a user wants to hold onto aWeak<Node>
for some reason.