So I'm just getting my feet wet with Go, and I'm trying to cleanly handle errors in the Go-way. One result is that having a type Movie struct
with methods to update the record, and sync to a data store. So an example method:
func (movie Movie) SetTitle(title string) : error {
prevTitle := movie.Title
movie.Title = title
json, err := json.Marshal(movie)
if (err != nil) {
movie.Title = prevTitle
return err
}
err = db.SetValue(movie.id, json)
if (err != nil) {
movie.Title = prevTitle
return err
}
return nil
}
The issue above is the 'cleanup' for a failed operation is to save the previous state, and restore as appropriate. But this feels very unmaintainable - it's so easy for a new error check to be made in the future, only to return without the proper cleanup. This would get even worse with multiple such 'cleanups'.
So to contrast, in other languages, I would wrap the whole bit in a try/catch, and handle cleanup just inside a single catch block. Here, I can:
movie
, do all the stuff on the copy, and then only copy back to the original object once everything is successfulif json, err := json.Marshal(movie); err == nil {
if err = db.SetValue(...); err == nil {
return nil
}
}
movie.Title = prevTitle;
return err
Which works, but I don't love having a level of nesting per check.
func Update() : err
function to minimize the number of checks needed (just thought of this - think I like this one)I feel like none of these are perfect; am I missing something obvious?
The way you are persisting can cause numerous problems:
I suggest, separate update and persistence. Playground
type Movie struct {
id int
Title string
}
func (m *Movie) Persist() error {
json, err := json.Marshal(m)
if err != nil {
return err
}
log.Printf("Storing in db: %s", json)
return nil
}
func main() {
m := &Movie{1, "Matrix"}
m.Title = "Iron Man"
if err := m.Persist(); err != nil {
log.Fatalln(err)
}
}
In your example you use by-value receiver. In this case, you get a copy of the struct in the method, you free to modify that copy, but all changes will not be visible outside.
func (movie Movie) SetTitleValueSemantic(title string) error {
movie.Title = title
json, err := json.Marshal(movie)
if err != nil {
return err
}
log.Printf("Serialized: %s", json)
return nil
}
playgorund: https://play.golang.org/p/mVnQ66TCaG9
I would strongly recommend avoiding such a coding style. In case you REALLY need some of this kind, here is an example for inspiration:
playground: https://play.golang.org/p/rHacnsRLkEE
func (movie *Movie) SetTitle(title string) (result *Movie, e error) {
movieCopy := new(Movie)
*movieCopy = *movie
result = movie
defer func() {
if e != nil {
result = movieCopy
}
}()
movie.Title = title
serialized, e := json.Marshal(movie)
if e != nil {
return
}
log.Printf("Serialized: %s", serialized)
return
}
Alexey is correct, but if the Movie
struct has pointer or slice fields, they wouldn't be copied.
Here's an example that manually copies the slice field (Tags
) before every update, and also features a nice transaction method (update
) I think you could use:
type Movie struct {
id int
Title string
Year int
Tags []string
}
func (m *Movie) update(fn func(m *Movie) error) error {
// Make a copy of Movie.
movieCopy := *m
// Manually copy slice and pointer fields.
movieCopy.Tags = make([]string, 0, len(m.Tags))
copy(movieCopy.Tags, m.Tags)
// Run the update transaction on the copy.
if err := fn(&movieCopy); err != nil {
return err
}
// Save to db.
data, err := json.Marshal(movieCopy)
if err != nil {
return err
}
return db.SetValue(m.id, data)
}
func (m *Movie) SetTitle(title string) error {
m.update(func(mm *Movie) error {
mm.Title = title
return nil
})
}
func (m *Movie) SetYear(year int) error {
m.update(func(mm *Movie) error {
mm.Year = year
return nil
})
}