5
\$\begingroup\$

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)
}
toolic
14.3k5 gold badges29 silver badges200 bronze badges
asked Apr 3 at 1:41
\$\endgroup\$

2 Answers 2

4
\$\begingroup\$
  1. Since FileStats is private to the package and not exported, you can rename it to fileStats. Similarly, you can set the member names of the struct to lowercase since they're not being exported.

  2. Instead of taking in *os.File as an argument to calculateStats, consider taking in an io.Reader (which *os.File implicitly satisfies). This allows you to pass in any object that implements the io.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 like strings.Reader or bytes.Buffer instead.

  3. 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)
    
  4. You don't need the os.Exit(0) at the end.

answered Apr 3 at 23:32
\$\endgroup\$
2
\$\begingroup\$
 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

answered Apr 8 at 15:44
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.