-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
"Remember" errors from setters, errWriter style #249
Comments
I think a generated func (s Person)Edit(fn func(pe *PersonEdit))error{
pe := &PersonEdit{
target: s,
errs: []error,
}
fn(pe)
if len(pe.errs) >0 {
return errors.Join(pe.errs...)
}
return nil
}
// and later
err := PersonVal.Edit(func(pe *PersonEdit){
pe.SetFirstName("foo")
pe.SetLastName("bar")
})
if err != nil{
// remediate
} |
After a bit of fiddling with this, I have a generated builder pattern I'd be interested in feedback on. Given the following schema
It generates code that allows for the following usage patterns: import "path/to/msg" // capnp-generated package
/* ... */
rootMsg, err := msg.NewRootMsg(seg)
if err != nil {
// handle error
}
// basic imperative with set at the end
person := rootMsg.BuildPerson()
person.Firstname("bob")
person.Lastname("everyman")
if err := rootMsg.Set(person); err != nil { // not actually necessary at current, see below
// handle error
}
// shiny fluent-style method-chaining
err = rootMsg.Set(rootMsg.BuildCost().Ram(5).Cpu(40))
if err != nil {
// handle error
} It works by generating an interface type per-container type and then implementations of this interface for each sub-struct. Currently, this is not necessary, since there's no need for the // Set added to containing struct type to take
// builder interface values
func (s Msg) Set(builder _MsgBuilder) error {
return builder._setMsg(s)
}
// interface for that particular struct type
// for any builders that target it. Type and
// interface method munged to try and
// avoid collision
type _MsgBuilder interface {
_setMsg(Msg) error
}
// Method added to struct type to
// spawn a new builder
func (s Msg) BuildCost() *_MsgNewCostBuilder {
b := &_MsgNewCostBuilder{}
b.v, b.err = s.NewCost()
// originally:
// b.v, b.err = s.Cost()
return b
}
// concrete type for the builder for Resources
type _MsgNewCostBuilder struct {
v Resources
err error
}
// implementation of the interface
func (b *_MsgNewCostBuilder) _setMsg(s Msg) error {
if b.err != nil {
return b.err
}
// originally:
// return b.SetCost(b.v)
return nil
}
// Setter method with abbreviated name and method chaining.
func (b *_MsgNewCostBuilder) Ram(v uint64) *_MsgNewCostBuilder {
if b.err == nil {
b.v.SetRam(v)
}
return b
}
/* OTHER METHODS AND TYPES */ Originally the idea with the interface was to avoid Would appreciate feedback. I am certainly not attached to abbreviated setters (e.g. Also worth noting that this has the side-effect of restricting builder methods to only field names in the schema instead of any extra methods on the generated type such as |
After mulling this over and going over the discussion in #64, which has a lot of overlap, I have a few different ideas, and was curious about the style and feasibility. Given the above schema, the normal setters: p, err := rootMsg.NewPerson()
if err != nil {
// handle error
}
err := p.SetFirstname("a")
if err != nil {
// handle error
}
err := p.SetFirstname("b")
if err != nil {
// handle error
}
c, err := rootMsg.NewCost()
if err != nil {
// handle error
}
c.SetCpu(1)
c.SetRam(2) Builder With
|
Per discussion starting at #64 (comment), handling errors for setters is easy to forget and doing it correctly is tedious. It might be nice if messages stored an error, in the style of errWriter: https://go.dev/blog/errors-are-values. This would allow users to check for errors from setters once, at the end.
The text was updated successfully, but these errors were encountered: