Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions students/mielofon/problems.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
5+5,10
1+1,2
8+3,11
1+2,3
8+6,14
3+1,4
1+4,5
5+1,6
2+3,5
3+3,6
2+4,6
5+2,7
90 changes: 90 additions & 0 deletions students/mielofon/quiz.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
package main

import (
"encoding/csv"
"errors"
"flag"
"fmt"
"os"
"strings"
"time"
)

type recordtype struct {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: I believe this should be recordType.

question string
answer string
}

// ReadStringWithLimitTime - function read string from reader with time limit
func ReadStringWithLimitTime(limit int) (string, error) {
timer := time.NewTimer(time.Duration(limit) * time.Second).C
doneChan := make(chan bool)
answer, err := "", error(nil)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a huge fan of this line here. What you are doing isn't wrong by any means, so this is a readability/style comment.

If it were me, I'd instead replace this with:

var answer string
var err error
// or
var (
	answer string
	err    error
)

Or even as a third alternative, you could use named return variables (I like this one less personally, but it is readable):

func ReadStringWithLimitTime(limit int) (answer string, err error) {

Yes, the first two options I presented are more code but I think it becomes clearer that you are just initializing them with zero values.

Building on that, you never actually use the err value. I mean, you return it on line 35:

return answer, err

But this is the only place you return it, and we know for certain that it is going to be nil at this point (at least I do after looking at the earlier code - it isn't as clear when reading line 35).

You could probably replace this line with var answer string then on line 35 do:

return answer, nil

Now it is clear you are returning a nil error in this case and you don't need that err variable anymore 😺

go func() {
fmt.Scanf("%s\n", &answer)
doneChan <- true
}()
for {
select {
case <-timer:
return "", errors.New("Timer expired")
case <-doneChan:
return answer, err
}
}
}

// ParseLines - parse lines from array of array of string to array of recordtype
func ParseLines(lines [][]string) []recordtype {
ret := make([]recordtype, len(lines))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Nice job allocating exactly how much memory you use. Its minor, but really cool to get in the habit of it when you know that info.

for i, line := range lines {
ret[i] = recordtype{
question: line[0],
answer: strings.TrimSpace(line[1]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that you trimmed up these strings 😄

}
}
return ret
}

func exit(msg string) {
fmt.Println(msg)
os.Exit(1)
}

func main() {

problemFileName := flag.String("csv", "./problems.csv", "a csv file in the format 'quastion,answer'")
limit := flag.Int("limit", 30, "the time limit for the quiz in seconds")
flag.Parse()

problemFile, err := os.Open(*problemFileName)
if err != nil {
exit(fmt.Sprintf("Failed to open the CSV file: %s\n", *problemFileName))
}

defer problemFile.Close() // close CSV file

readerProblem := csv.NewReader(problemFile)
lines, err := readerProblem.ReadAll()
if err != nil {
exit("Failed to parse the provided CSV file.")
}

problems := ParseLines(lines)

successAnswerCount := 0
for i, p := range problems {
fmt.Printf("Problem #%d: %s=", i+1, p.question)

answer, err := ReadStringWithLimitTime(*limit)
if err != nil {
println("Time expire!")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Stick to fmt.Println instead of println. The latter was added to the language as part of the bootstrapping process, but may be removed from the language at some point breaking code like this, so the former (fmt.Println) is preferred. Source: https://golang.org/ref/spec#Bootstrapping

break
}
if strings.ToLower(strings.Trim(answer, "\n ")) == p.answer {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you need the \n part of this, and you may want to check out TrimSpace. What you have works, but TrimSpace is just a nice helper to get all kinds of whitespace.

On another note - you do ToLower here, but you DO NOT do that to input from the CSV. I would consider adding a call to ToLower in your ParseLines function to guarantee that both user input and csv input are both in the same case.

successAnswerCount++
}
}
println("You scored", successAnswerCount, "out of", len(problems))

}