Ytdyklly

This is my first Go program, would like to know what could be improved, what is done wrong, and anything else that I should know. The CSV contains 10 questions with the 10 answers separated by a comma. For example:

10+10,20

4+5,32

If you want the full context this is from gophercises which are some exercises to practice Go.

package main



import (

    "encoding/csv"

    "flag"

    "fmt"

    "math/rand"

    "os"

    "strings"

    "time"

)





var points int



func main() {

    fPtr := flag.String("csv", "problems.csv", "FileName in csv (question, answer)")

    tPtr := flag.Int("time", 10, "Time in seconds")

    sPtr := flag.Bool("shuffle", true, "shuffle your questions order")

    flag.Parse()

    questions := readCSV(*fPtr)



    fmt.Print("Press enter to start you have ", *tPtr, " seconds")

    fmt.Scanln()



    askQuestions(&questions, *tPtr, *sPtr)



    fmt.Println("You got ", points, " points")

}



func askQuestions(questions *string, time int, s bool) {



    go startTimer(time)

    for _, i := range shuffle(s, len(*questions)) {

        fmt.Println("Question ", (*questions)[i][0], ":")

        var a string

        fmt.Scan(&a)

        a = strings.ToLower(strings.Trim(a, " "))

        if a == (*questions)[i][1] {

            points++

        }

    }

}



func shuffle(shuffle bool, sliceLen int) int {

    var r int

    for i := 0; i < sliceLen; i++ {

        r = append(r, i)

    }

    if shuffle {

        rand.Seed(time.Now().Unix())

        rand.Shuffle(len(r), func(i, j int) {

            r[i], r[j] = r[j], r[i]

        })

    }



    return r

}



func readCSV(s string) (questions string) {

    f, err1 := os.Open(s)

    records, err2 := csv.NewReader(f).ReadAll()

    for err1 != nil || err2 != nil {

        fmt.Println("Error: ", err1, err2)

        fmt.Println("Please re-enter the name of the CSV file: ")

        fmt.Scan(&s)

        f, err1 = os.Open(s)

        records, err2 = csv.NewReader(f).ReadAll()

    }

    return records

}



func startTimer(seconds int) {

    time.Sleep(time.Duration(seconds) * time.Second)

    fmt.Println("Time is up! You got ", points, " points")

    os.Exit(0)

}

startTimer()

The time package provides timers natively. So rather than using a call to time.Sleep(), you can use time.NewTimer().

Use fmt.Printf() rather than fmt.Println() for printing formatted strings.

func startTimer(seconds int) {

    time.Sleep(time.Duration(seconds) * time.Second)

    fmt.Println("Time is up! You got ", points, " points")

    os.Exit(0)

}

Becomes:

func startTimer(sec int) {

    timer := time.NewTimer(time.Duration(sec) * time.Second)

    <-timer.C

    fmt.Printf("Time is up! You got %d pointsn", points)

    os.Exit(0)

}

readCSV()

In readCSV(), you have multiple error variables. You can instead use a single error variable as you go along.

I advise against adding file retry logic. It complicates the code, and most users can simply re-run the command upon error. Instead, you should return an error value.

Using a named return (questions) means you should actually use the questions variable. Because I switch to multiple return values, I removed the named parameter.

Also be sure to close the file after you're done reading from it.

func readCSV(s string) (questions string) {

    f, err1 := os.Open(s)

    records, err2 := csv.NewReader(f).ReadAll()

    for err1 != nil || err2 != nil {

        fmt.Println("Error: ", err1, err2)

        fmt.Println("Please re-enter the name of the CSV file: ")

        fmt.Scan(&s)

        f, err1 = os.Open(s)

        records, err2 = csv.NewReader(f).ReadAll()

    }

    return records

}

Becomes:

func readCSV(name string) (string, error) {

    f, err := os.Open(name)



    if err != nil {

        return nil, err

    }



    qs, err := csv.NewReader(f).ReadAll()



    if err != nil {

        return nil, err

    }



    return qs, f.Close()

}

askQuestions()

Again, here you should use fmt.Printf() to print formatted strings. You also use the value i as an index, but with the range keyword you can access the currently indexed value, see here.

You should check the return of fmt.Scan().

Rather than starting the timer within askQuestions(), I opted to move it to main(). This means we can remove the time argument.

main()

Naming variables like fPtr and sPtr is just extra typing. Documenting the type in the name itself is not very useful.

• 
  fPtr becomes name
  


• 
  tPtr becomes time
  


• 
  sPtr becomes shuffle
  

From your updated code with shuffling:

Rather than keeping a second copy of the shuffled questions, just modify the array in-place. You also don't need to seed rand (see this example from the Go docs).

Conclusion

Here is the code I ended up with:

package main



import (

    "encoding/csv"

    "flag"

    "fmt"

    "log"

    "math/rand"

    "os"

    "time"

)



var points int



func main() {

    name := flag.String("csv", "problems.csv",

        "filename in csv (question, answer)")

    time := flag.Int("time", 10, "time in seconds")

    shuffle := flag.Bool("shuffle", true, "shuffle your question order")



    flag.Parse()



    questions, err := readCSV(*name)



    if err != nil {

        log.Fatal(err)

    }



    if *shuffle {

        rand.Shuffle(len(questions), func(i, j int) {

            questions[i], questions[j] = questions[j], questions[i]

        })

    }



    go startTimer(*time)



    if err := askQuestions(&questions); err != nil {

        log.Fatal(err)

    }



    fmt.Printf("You got %d pointsn", points)

}



func readCSV(name string) (string, error) {

    f, err := os.Open(name)



    if err != nil {

        return nil, err

    }



    qs, err := csv.NewReader(f).ReadAll()



    if err != nil {

        return nil, err

    }



    return qs, f.Close()

}



func askQuestions(questions *string) error {

    for i, q := range *questions {

        fmt.Printf("Question %d: %sn", i+1, q[0])



        var a string

        _, err := fmt.Scan(&a)



        if err != nil {

            return err

        }



        if a == q[1] {

            points++

        }

    }



    return nil

}



func startTimer(sec int) {

    timer := time.NewTimer(time.Duration(sec) * time.Second)

    <-timer.C

    fmt.Printf("nTime is up! You got %d pointsn", points)

    os.Exit(0)

}

Hope this helps!