cloudevents / sdk-go

Go SDK for CloudEvents
https://cloudevents.github.io/sdk-go/
Apache License 2.0
786 stars 217 forks source link

Immutability? #250

Open timbray opened 4 years ago

timbray commented 4 years ago

A bit late to the game here, but was working with an internal AWS API for our AWS Events - a whole lot like CloudEvents - and the API has all the getters you'd expect just like this, but no setters because in our world an AWS event is immutable. There's a separate AWSEventBuilder type that has WithTime(), WithType(), etc etc etc and then finally Build().

There's a lot of code that depends on immutability, e.g. using the ID field for deduping and so on. Having the API allow mutation makes me nervous.

n3wscott commented 4 years ago

You can not block that in Golang. You can always do something like event.Context.ID = "new-id".

timbray commented 4 years ago

I still think the API is sending the wrong message. Immutability is an important virtue and for immutable types, it's typical to have a builder and getters, but no setters.

n3wscott commented 4 years ago

I am open to the idea of introducing a a builder or factory for events. I started that route and we yanked it because it was not very go-ish. But, nether are setters/getters.

The setter/getters are really there because no generics in go mean accessing versioned members are hard to access. The getter/setters are to help with that indirection.

Do you have an api you would like to see for a builder? We are building and planning a v2 SDK now with some cleaner designs.

slinkydeveloper commented 4 years ago

Given all the facilities the Event data structure now has, like Clone(), setters, getters, I feel the design went quite away from immutability.

On the other side, a builder could be an interesting feature even if, as scott pointed out, it's not really idiomatic of golang. But we can definitely give it a try. @timbray are you willing to provide a PR for that?