8
\$\begingroup\$

In order to learn Go, I've implemented the cat program in Go. I want to know if the code is idiomatic and what could be improved.

package main

import "fmt"
import "io"
import "os"

var errors int

func main() {
    catFilesOrStdin(os.Args[1:])
    os.Exit(getExitValue())
}

func getExitValue() int {
    if (hadErrors()) {
        return 1
    } else {
        return 0
    }
}

func hadErrors() bool {
    return errors > 0
}

func catFilesOrStdin(files []string) {
    if len(files) > 0 {
        catFiles(files)
    } else {
        catStream(os.Stdin)
    }
}

func catFiles(files []string) {
    for _,file := range files {
        catFile(file)
    }
}

func catFile(file string) {
    stream, err := os.Open(file)
    if err != nil {
        fmt.Fprintln(os.Stderr, err)
        errors++
        return
    }
    catStream(stream)
    defer stream.Close()
}

func catStream(stream *os.File) {
    _, err := io.Copy(os.Stdout, stream)
    if err != nil {
        fmt.Fprintln(os.Stderr, err)
        errors++
    }
}
\$\endgroup\$
2
  • 2
    \$\begingroup\$ I think perhaps you're going overboard with the number of functions. I also don't see a reason not to make it func catStream(r io.Reader) rather than require a *os.File. \$\endgroup\$
    – Dave C
    Commented Aug 13, 2015 at 14:28
  • \$\begingroup\$ Good point about the io.Reader instead of os.File, I agree. \$\endgroup\$ Commented Aug 13, 2015 at 15:44

1 Answer 1

4
\$\begingroup\$

I'd put the defer statement in catFile above the call to catStream, not after.

You might want to consider log.Fatal if the os.Open fails.

If you're importing multiple packages, the standard is a single import statement with all the packages in parentheses.

The global errors variable technically should be protected with a sync.Mutex, but more importantly, log.Fatal on any error instead.

\$\endgroup\$
2
  • 1
    \$\begingroup\$ wrt log.Fatal on any error, although I'd probably do that if implementing something like this from scratch, that is not the behaviour of unix cat. E.g. date | cat /nosuchfile - gives a warning message and continues on (but exits with 1 instead of 0); this is the behaviour being implemented here. \$\endgroup\$
    – Dave C
    Commented Aug 13, 2015 at 14:26
  • 2
    \$\begingroup\$ Although, I would use the log package. Something like log.SetPrefix("cat: "); log.SetFlags(0) then log.Println(err) rather than explicitly fmt.Fprintln to os.Stderr. But that's more of a preference thing. You could probably wrap warning logging into a function that also did errors++. \$\endgroup\$
    – Dave C
    Commented Aug 13, 2015 at 14:29

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.