Skip to content

Implement "ObserverRef" to avoid explicitly referring to observer types #2064

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

Closed
addisoncrump opened this issue Apr 17, 2024 · 2 comments
Closed
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@addisoncrump
Copy link
Collaborator

addisoncrump commented Apr 17, 2024

We saw this here: #1886

Basically, to access an observer, we currently have to explicitly refer to the type of the observer when fetching it with match_name and friends. We can sidestep this by defining an ObserverRef type, e.g.:

#[derive(Copy, Debug, Serialize, Deserialize)]
pub struct ObserverRef<T>(PhantomData<T>);

Then defining (default) in Observer:

pub fn make_reference(&self) -> ObserverRef<Self> {
  ObserverRef(PhantomData)
}

And finally defining a MatchNameRef blanket:

pub trait MatchNameRef {
  fn match_name_by_ref<T>(&self, name: &str, ref: ObserverRef<T>) -> Option<&T>;
}

impl<M> MatchNameRef for M where M: MatchName {
  fn match_name_by_ref<T>(&self, name: &str, ref: ObserverRef<T>) -> Option<&T> {
    self.match_name::<T>(name);
  }
}

In this way we can avoid ever explicitly referring to type names, since the type will be encoded in the ObserverRef wrapper and inferred.

@addisoncrump addisoncrump added enhancement New feature or request good first issue Good for newcomers labels Apr 17, 2024
@addisoncrump
Copy link
Collaborator Author

In fact, we have this pattern in a few places (e.g. metadata) so maybe it would be better to define TypeRef and just make it a blanket impl (impl<T> TypeRef for T).

@domenukk
Copy link
Member

I think if we're doing this, we can have the ref also include the name, then we have a real 1:1 mapping of observer to the match, right? That'd be amazing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants