Command called 'sprinkle'

Question

I have a command called sprinkle. I'm not going to explain anything about it, because I want whoever that reviews the code to also tell me how much of the command documentation they understood.

Here is what I need help with:

• Is the documentation good enough? Is it too verbose?
• Does the constant domainName help you understand the code below, or is it useless?
• Should the transformations array be passed around instead of being defined as a global variable?
• What else can I improve in the code?

// The sprinkle command allows the user to define some transformations in a file called test.txt.
// The user can then provide some domain names such as 'go' and the application will randomly transform it, using the transformations defined in test.txt.
// Every transformation in the test.txt file must have an asterisk (*) in place of the domain name. For example, if the test file contains:
//
// *-is-awesome.com
//
// love-*.net
//
// lets-*.org
// 
// Then, if the user enters 'go' in the command line, the output could be something like:
//
// go-is-awesome.com
package main
import (
 "bufio"
 "fmt"
 "log"
 "math/rand"
 "os"
 "strings"
 "time"
)
const (
 domainName = "*"
)
var transformations []string
func main() {
 var err error
 transformations, err = readLinesFromFile("test.txt")
 if err != nil {
 log.Fatal(err)
 }
 inputScanner := bufio.NewScanner(os.Stdin)
 for inputScanner.Scan() {
 fmt.Println(applyRandomTransformation(inputScanner.Text()))
 }
}
// readLinesFromFile reads a file into an array. Each element in the array is one line of the text file.
func readLinesFromFile(path string) ([]string, error) {
 var lines []string
 file, err := os.Open("test.txt")
 if err != nil {
 return nil, err
 }
 defer file.Close()
 scanner := bufio.NewScanner(file)
 for scanner.Scan() {
 lines = append(lines, scanner.Text())
 }
 return lines, nil
}
// applyRandomTransformation randomly chooses a transformation from the list of transformations, and replaces the '*' in the transformation with input
func applyRandomTransformation(input string) string {
 rand.Seed(time.Now().UnixNano())
 transformation := transformations[rand.Intn(len(transformations))]
 return strings.Replace(transformation, domainName, input, -1)
}

Answer 1

Apart from what @AmirhoseINZif has answered, there are two other items that I feel are significant.

1. You pass in the file-name "test.txt" when you call the transformations, err = readLinesFromFile("test.txt"), and that becomes the path parameter. But, you don't use that at all, your actual file-open is: file, err := os.Open("test.txt"). Use the path!
2. You re-seed the rand generator each time you call the transform method. You only need to do that once. Seed it in your main method, or even as an init function instead.

Apart from those issues, it's great to see you doing things like:

• defer file.Close() - that's the right strategy.
• your variable names are mostly really clear, and useful.
• the log.Fatal(err) is effective error handling.

Update: About the transformation as a global...

transformation is a slice, not an array. As a slice, it consists of a lenght, capacity, and a pointer to an array. Let's assume each of those are 8 bytes, so that's 24 bytes of data that a slice represents. It is not a problem to copy that data as a parameter in to the applyRandomTransformation method. Read up about slices here: effective go

Eseentially your performance concerns, or memory concerns, are not an issue when copying the slice, because you are not actually copying the array, just a pointer to the array.

• Parham Doustdar: Since you agree with @AmirhoseiN Zlf about not using a global variable, I will need to create the transformations variable inside the main() function. Then, I have two options: pass in the empty transformations array as the second argument by reference to readLinesFromFile() and fill it rather than returning an array, or pass transformations by reference to applyRandomTransformation() by reference? Or, maybe this is just microoptimization? I still have trouble deciding on pointers vs no pointers. Can you please update your answer to explain your agreement more?
• rolfl: @ParhamDoustdar see update.

Answer 2

First, look at your documentation to see how will look to users:

The Go Blog

Godoc: documenting Go code

$ go doc sprinkle
The sprinkle command allows the user to define some transformations in a
file called test.txt. The user can then provide some domain names such as
'go' and the application will randomly transform it, using the
transformations defined in test.txt. Every transformation in the test.txt
file must have an asterisk (*) in place of the domain name. For example, if
the test file contains:
*-is-awesome.com
love-*.net
lets-*.org
Then, if the user enters 'go' in the command line, the output could be
something like:
go-is-awesome.com
$

Amongst other things, domain name is a misnomer.

Second, look at your sprinkle command help:

$ ./sprinkle -help
$

Third, read your code. And, fourth, run your code.

Find and fix several bugs (others have identified some of them). Since domainName is a misnomer, rename it to wildcard.

Rewrite the sprinkle command go doc description:

$ go doc 
The sprinkle command transforms a random line of text from the sprinkle.txt
file by replacing asterisk (*) wildcard characters with text from stdin.
For example,
 $ cat sprinkle.txt
 *-is-awesome.com
 love-*.net
 lets-*.org
 $ echo go | ./sprinkle
 love-go.net
 $ echo go | ./sprinkle
 go-is-awesome.com
 $
$

TODO: provide sprinkle command help.

• Parham Doustdar: Thanks! This command was an exercise from the "Go Programming Blueprints" book. It's astounding to see how much knowledge I had left out of my documentation because I was focused on solving a particular problem. Your review taught me something irrelevant to any programming language.

Answer 3

I don't know so much about go, but I write things that my help you.

I think it's better to assign applyRandomTransformation result to a variable in

 fmt.Println(applyRandomTransformation(inputScanner.Text()))

passing transformations is much better than define it as a global, global variables are always dangerous.

I don't understand domainName.

• Parham Doustdar: I would love to have it not be a global variable. However, how else can I not have two copies of the transformations array? Is it okay to pass the whole transformations array to different functions when it is too big, maybe with 1000 elements?