`defer` is really not well-suited for error handling, its benefit is mainly in resource cleanup where failure is impossible or doesn't matter. (This makes it fine for `Close` on read-only file I/O operations, and not so great for writes.)
`defer` is really not well-suited for error handling, its benefit is mainly in resource cleanup where failure is impossible or doesn't matter. (This makes it fine for `Close` on read-only file I/O operations, and not so great for writes.)
Can you give an example case of how this could happen?
func UpdateUser(user *User) (err error) {
f, err := os.Create("/some/file.txt")
if err != nil {
return err
}
defer CheckClose(f, &err)
if _, err := f.Write(somedata); err != nil {
return err
}
if err := db.UpdateUser(user, "/some/file.txt"); err != nil {
return err
}
return
}
This function might have updated the user in the database with a new file despite the fact that `CheckClose` (defined up-thread) does check to see if the `Close` failed and returned an error. The calling code won't have known this has happened.The core problem is that the error checking is not done soon enough, either because Go programmers are conditioned to `defer f.Close()` from nearly all example code -- most of it demonstrating reads, not writes -- or because they are handling the error, but only in a deferred function, not earlier.
A more correct way to do this would be:
func UpdateUser(user *User) error {
f, err := os.Create("/some/file.txt")
if err != nil {
return err
}
defer f.Close()
if _, err := f.Write(somedata); err != nil {
return err
}
if err := f.Sync(); err != nil {
return err
}
if err := f.Close(); err != nil {
return err
}
if err := db.UpdateUser(user, "/some/file.txt"); err != nil {
return err
}
}
`Sync()` flushes the data to disk, and `Close()` gives a "last-chance" opportunity to return an error. The `defer f.Close()` exists as a way to ensure resource cleanup if an error occurs before the explicit `f.Close()` toward the end of the function. As I mentioned in an update to the post, double `Close()` is fine.Then you can use defer in the file-writing function if you so please, and not bother to close at the end explicitly, without issue. A more robust example might be to even include the sync call in the deferred function (and even clean up the file itself on error). To re-use your example from your blog post:.
func helloNotes() (err error) {
var f *os.File
f, err = os.Create("/home/joeshaw/notes.txt")
if err != nil {
return
}
defer func() {
if err == nil {
err = f.Sync()
}
cerr := f.Close()
if err == nil {
err = cerr
}
if err != nil {
os.Remove("/home/joeshaw/notes.txt")
}
}()
err = io.WriteString(f, "hello world")
return
}
I would probably move that out into a helper, though, so I could do something like defer SafeClose(f, &err)
instead, and be able to use it elsewhere. Hell, even without defer, it's nice to have a helper that will sync and close for you so you can avoid the boilerplate, if you have lots of different bits of code that writes files.FWIW, I'm not sure why you are so negative on named return values, but I'm at best a novice Go programmer, so perhaps I don't fully understand why they aren't great (I guess it does look weird to me to have bare `return` statements that do actually return a value even though it doesn't look like it). Your argument about the return value possibly being modified after the core function finishes being unintuitive doesn't really strike me as a big deal either.
Defer is by definition the last work you do in a function, there won't be more work except by the caller who will get the error returned to them.
If you are structuring a function that writes a file, and then does something with it, defer isn't appropriate, since you should close it before you do any more work.