sdk-go icon indicating copy to clipboard operation
sdk-go copied to clipboard

Immutability?

Open timbray opened this issue 5 years ago • 4 comments

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.

timbray avatar Oct 31 '19 17:10 timbray

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

n3wscott avatar Nov 01 '19 15:11 n3wscott

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.

timbray avatar Nov 01 '19 18:11 timbray

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.

n3wscott avatar Nov 07 '19 17:11 n3wscott

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?

slinkydeveloper avatar May 20 '20 08:05 slinkydeveloper