Skip to content

Conversation

@viveksyngh
Copy link
Contributor

Signed-off-by: Vivek Singh vivekkmr45@yahoo.in

added my solution

Signed-off-by: Vivek Singh <vivekkmr45@yahoo.in>

added my solution

fixed issue with blocking input

added my solution

Signed-off-by: Vivek Singh <vivekkmr45@yahoo.in>

added my solution
@joncalhoun joncalhoun added the in review PR/Issue is currently undergoing code review label Mar 23, 2018
Copy link

@arsham arsham left a comment

Choose a reason for hiding this comment

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

Awesome coding skills.

"math/rand"
)

type Question struct {
Copy link

Choose a reason for hiding this comment

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

It's a good idea to document an exported struct. You can use golint tool to tell you about these actions:

golint students/viveksyngh

if(err != nil) {
log.Fatal("Failed to parse CSV file.")
}
questions := make([]Question, 0)
Copy link

Choose a reason for hiding this comment

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

Now you have successfully loaded your questionList, you have the required length. You can do this instead:

questions := make([]Question, len(questionList))
for i, question := range questionList {
	if len(question) != 2 {
		continue
	}
	questions[i] = Question{
		strings.TrimSpace(question[0]),
		strings.TrimSpace(question[1]),
	}
}

This way you don't allocate new array every time append is called. Also always check the input coming from outside world.

return questions
}

func Quiz(questions []Question, timer *time.Timer) (score int){
Copy link

Choose a reason for hiding this comment

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

There is no need for named return, it's another allocation. If you simply name your function score it informs the reader what they expect.

}()

select {
case <-timer.C:
Copy link

Choose a reason for hiding this comment

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

Very clever use of timers.

return score
case userAnswer := <- answerChannel:
if(strings.TrimSpace(userAnswer) == question.answer) {
score += 1
Copy link

Choose a reason for hiding this comment

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

You can simply do: score++

}

func randomize(questions []Question) []Question{
n := len(questions)
Copy link

Choose a reason for hiding this comment

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

You can do this too:

rand.Seed(time.Now().UnixNano())
for i := range questions {
	j := rand.Intn(i + 1)
	questions[i], questions[j] = questions[j], questions[i]
}

or

rand.Seed(time.Now().UnixNano())
rand.Shuffle(len(questions), func(i, j int) {
	questions[i], questions[j] = questions[j], questions[i]
})

It's a good idea to seed your random generator otherwise the shuffle returns the same results every time you run the program, which defeats its purpose.

@joncalhoun joncalhoun merged commit 6ef43f9 into gophercises:master Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in review PR/Issue is currently undergoing code review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants