Reputation: 54193
If I'm opening a file inside a for
loop and will be finished with it at the end of that iteration, should I call Close
immediately or trick Defer
using a closure?
I have a series of filenames being read in from a chan string
which have data to be copied into a zipfile. This is all being processed in a go func.
go func(fnames <-chan string, zipfilename string) {
f, _ := os.Create(zipfilename) // ignore error handling for this example
defer f.Close()
zf := zip.NewWriter(f)
defer zf.Close()
for fname := range fnames {
r, _ := os.Open(fname)
w, _ := zf.Create(r.Name())
io.Copy(w, r)
w.Close()
r.Close()
}(files, "some name.zip")
Inside my for
loop, would it be more idiomatic Go to write:
for fname := range fnames {
func(){
r, _ := os.Open(fname)
defer r.Close()
w, _ := zf.Create(r.Name())
defer w.Close()
io.Copy(w, r)
}()
}
or should I continue with my code as-written?
Upvotes: 0
Views: 2487
Reputation: 53418
You should be checking your errors. I know this is meant to just be an example, but in this case it is important. If all you do is defer Close(), you can't actually check if there was an error during defer.
The way I would write this is to create a helper function:
func copyFileToZip(zf *zip.Writer, filename string) error {
r, err := os.Open(filename)
if err != nil {
return err
}
defer r.Close()
w, err := zf.Create(r.Name())
if err != nil {
return err
}
defer w.Close()
_, err = io.Copy(w, r)
if err != nil {
return err
}
return w.Close()
}
Once you add in all that error handling, the function is big enough to make it a named function. It also has the added benefit of checking the error when closing the writer. Checking the reader's error is unnecessary since that won't affect if the data was written.
Upvotes: 2