TLDR: First time dev in Go moving from Java and I need feedback on my simple CLI Tool.
I wanted to get competent in Go as a very junior developer, so this is my first basic project in go. I set a goal for myself to write a command-line tool that reads a text file and calculates basic statistics using only the standard libraries and documentation as my guide. I've never written a CLI tool and I have no experience in GO so I want some feedback on my implementation. My background is in Java, so I'm guessing I've probably done a few things oddly by go standards. Any critique is appreciated, especially Go specific critique with the goal of learning "idiomatic" go.
package main
import (
"bufio"
"fmt"
"os"
"strings"
)
/*
Objective:
Build a command-line tool that reads a text file and calculates basic statistics like:
Word count
Line count
Character count
Most frequent words (Need to do)
*/
/*
Core Features
Read a File
Use os.Open and bufio.Scanner to read input.
Calculate Stats
Use slices, maps, and loops to process text.
Display Results
Format output with fmt or flag for CLI args.
*/
type FileStats struct {
Lines int
Words int
Chars int
}
func calculateStats(file *os.File) FileStats {
stats := FileStats{}
scanner := bufio.NewScanner(file)
for scanner.Scan() {
line := scanner.Text()
stats.Lines++
stats.Words += len(strings.Fields(line)) // Split line into words
stats.Chars += len(line) + 1 // +1 for newline character
}
return stats
}
func main() {
//Startup happens once no need to make into a function
if len(os.Args) < 2 {
fmt.Println("Usage: go run main.go <filename>")
os.Exit(1)
}
filename := os.Args[1]
file, err := os.Open(filename)
fmt.Println(filename)
if err != nil {
fmt.Printf("Error opening file: %v\n", err)
os.Exit(2)
}
stats := calculateStats(file)
err = file.Close()
if err != nil {
fmt.Printf("Error closing file: %v\n", err)
os.Exit(3)
}
fmt.Printf("Lines: %d\nWords: %d\nChars: %d\n", stats.Lines, stats.Words, stats.Chars)
os.Exit(0)
}
2 Answers 2
Since
FileStats
is private to the package and not exported, you can rename it tofileStats
. Similarly, you can set the member names of the struct to lowercase since they're not being exported.Instead of taking in
*os.File
as an argument tocalculateStats
, consider taking in anio.Reader
(which*os.File
implicitly satisfies). This allows you to pass in any object that implements theio.Reader
interface, not just*os.File
. For example, if you want to write a unit test for this function, you don't have to create a file and pass it in. Instead, you can use something likestrings.Reader
orbytes.Buffer
instead.You should try to use
defer
when cleaning up resources (or any other action that needs to happen at the end of the function call, like unlocking a mutex).file, err := os.Open(filename) if err != nil { // error } defer file.Close() stats := calculateStats(file)
You don't need the
os.Exit(0)
at the end.
line := scanner.Text()
...
stats.Chars += len(line) + 1 // +1 for newline character
Not +1 precisely. Docs of bufio.Split are not verbose at all, but succinctly note that "The default split function is ScanLines." and docs of bufio.ScanLines say:
The end-of-line marker is one optional carriage return followed by one mandatory newline. In regular expression notation, it is
\r?\n
.
So, sometimes +2 characters.
Also, it can be +0 characters for the very last iteration, if the file ends with a non-newline character.
I would read file length with os
package, the bufio.Scanner won't be any good for that.
Next thing:
fmt.Println("Usage: go run main.go <filename>")
This is a bit uncommon. Since go compilation (cross-compilation) rocks, users are more likely to interact with binaries. The go run .
is harder - that person has to have go
installed and has to be able to determine the path of main
. It's assumed that if they know that, they would figure out go run .
by themselves. (And the go run main.go
is less often used because of its pitfalls.)
Example format recommended by a popular Go library (https://github.com/spf13/cobra/blob/ceb39aba25c86233e4888210c4b57cdb0a78d1e1/command.go#L63):
add [-F file | -D dir]... [-f format] profile