Hi, thank you for the helpful comments. That's what I love about the Go community.
On Thu, Mar 04, 2021 at 03:14:04PM +0100, 'Axel Wagner' via golang-nuts wrote: > - Who's responsible for doing that work? If I return *os.File directly > from getFile(), there's an implicit assumption that the file needs to be > closed by the caller. But what if it's os.Stdout? > > > One possibility that we need to mention is that it would be possible to return > a new file, connected to stdout, for example by using syscall.Dup. You could > also wrap `os.Stdout` in a new type, with a no-op `Close` method. I actually hadn't thought of that. So instead of a Writer and a cleanup function I could just as easily return an io.WriteCloser. This would make it explicit, that the caller is supposed to call Close. A custom type (wrapping the actual writer) with a Close method can then just return nil in case there's nothing to be done. > - The alternative of putting all that in run() is kinda ugly because > run() potentially has to do a lot more. > > Nevertheless, personally, I think it's the right choice, ultimately. In small programs I'd actually do that. But in the real case that inspired this small example run() has to do a lot of setup work and I'd rather like to keep it clean by encapsulating the various steps in separate functions. > A potential problem I see? The error from os.File.Close() is never > checked, neither is the error from bufio.Writer.Flush(). In this small > example that wouldn't bother me because it means the program ends > anyway. But in other cases it might become more of a problem. > > > Yes, at the very least, your cleanup function should return an `error`. That > `error` might just be statically `nil`, but the failure when flushing/closing > (FTR, I don't think you need to flush, if you close the file anyway) should > definitely be handled some way. That's why I think your code as-is is actually > buggy. "The program will end anyway" isn't a good reason to silently leave a > corrupt file behind. I agree. By using a wrapping type and returning an io.WriteCloser that's actually possible. Side note about flushing: I'm flushing the wrapping bufio.Writer there. I'd be surprised closing the underlying os.File would be enough to ensure all buffer contents have been written. Thanks for the feedback. Chris -- You received this message because you are subscribed to the Google Groups "golang-nuts" group. To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts+unsubscr...@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/YED8ezxQZPxB6hls%40junction.dune.
