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

Proposal to allow destructuring of Event #243

Open
ozabalaferrera opened this issue Nov 14, 2024 · 6 comments
Open

Proposal to allow destructuring of Event #243

ozabalaferrera opened this issue Nov 14, 2024 · 6 comments

Comments

@ozabalaferrera
Copy link
Contributor

I noticed this comment in the message tests:
https://github.com/cloudevents/sdk-rust/blob/main/src/event/message.rs#L185

 #[test]
    fn message_v03_roundtrip_binary() -> Result<()> {
        //TODO this code smells because we're missing a proper way in the public APIs
        // to destructure an event and rebuild it

So I made some changes to support destructuring an Event:
https://github.com/ozabalaferrera/cloudevents-sdk-rust/compare/oz_upgrade_all...ozabalaferrera:cloudevents-sdk-rust:oz_add_destructuring?expand=1

@jcrossley3, do you think this is a worthwhile change? I ran into situations in the past where I wanted to destructure, but didn't think much of it until I saw the test comments. I believe this change shouldn't break anything.

@jcrossley3
Copy link
Contributor

I'm sure I'm missing something, but what's the PhantomData for?

@ozabalaferrera
Copy link
Contributor Author

I'm sure I'm missing something, but what's the PhantomData for?

I needed a zero-sized type for the private field. I'm used to PhantomData from doing type-state stuff, but the unit type () alone works just as well. Thanks for questioning that; I updated my repo's branch.

@jcrossley3
Copy link
Contributor

I guess I still don't understand. If you remove the _private field, what happens?

@ozabalaferrera
Copy link
Contributor Author

Oh, I misunderstood your question, sorry. I added pub(crate) _private so that the struct has a private (outside the crate) field to prevent direct, field initialization of the struct (I forgot the term for this). The other fields can then be public to allow destructuring. This retains the public API, forcing the use of existing methods for creating the struct.

@jcrossley3
Copy link
Contributor

Ah, now I understand. For me, the value of preventing direct, field initialization of the struct isn't worth the "type noise" of
requiring the double-dot operator to destructure the Event. To borrow a Python term, I'm in the "consenting adults" camp. I'm ok making the fields public if it simplifies the calling code.

@ozabalaferrera
Copy link
Contributor Author

I figured those fields were private very intentionally since there are builders to ensure the constructed Event is valid per the CloudEvent specification. Maybe @Lazzaretti can weigh in.

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

No branches or pull requests

2 participants