Skip to content
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

Add consistent way of initializing custom event kinds #52

Closed
ok300 opened this issue Feb 24, 2023 · 5 comments
Closed

Add consistent way of initializing custom event kinds #52

ok300 opened this issue Feb 24, 2023 · 5 comments
Labels
enhancement New feature or request

Comments

@ok300
Copy link
Contributor

ok300 commented Feb 24, 2023

Describe the enhancement

It's possible to construct event kinds that make them incompatible with (i.e. not == to) the json deserialized version of themselves.

The enhancement would be to only expose valid ways of constructing event kinds, such that this is not an issue anymore.

Use case

For example, for a custom kind 20100, which is in the ephemeral range:

// Will NOT work (false)
if let received_event.kind == Kind::Custom(20100) { }

// Will work (true)
if let received_event.kind == Kind::Ephemeral(20100) { }

// Will work (true)
// Looks like the most appropriate way
if let received_event.kind == Kind::from(20100) { }

Additional context

The only way I can think of is to enforce kinds to only be constructed via Kind::from(u64), although not sure how to do that.

@ok300 ok300 added the enhancement New feature or request label Feb 24, 2023
@yukibtc
Copy link
Member

yukibtc commented Feb 24, 2023

Maybe the best way is to impl Eq and PartialEq manually checking if the u64 kind number is equal.
In this way Kind::Custom(20100) will be equal to Kind::Ephemeral(20100).
What do you think?

@yukibtc
Copy link
Member

yukibtc commented Feb 24, 2023

Something like:

impl PartialEq<Kind> for Kind {
    fn eq(&self, other: &Kind) -> bool {
        self.as_u64() == other.as_u64()
    }
}

@ok300
Copy link
Contributor Author

ok300 commented Feb 24, 2023

Sounds promising.

@yukibtc
Copy link
Member

yukibtc commented Feb 24, 2023

Now I have the doubt to have misunderstood what you mean.
Have I?

I don't think that can be enforced the use of Kind::from(...).

Using the solution that I applied, if someone used Kind::Custom instead Kind::Ephemeral everything works (I added ephemeral kind not long ago). In this way it's backwards compatible.

@ok300
Copy link
Contributor Author

ok300 commented Feb 24, 2023

Tested the new version, works! Thanks.

You're right, your idea with PartialEq is even better than enforcing Kind::from(...). Especially cause it's backwards compatible.

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

No branches or pull requests

2 participants