Swarna's solutions for Quiz and Url shortener#4
Swarna's solutions for Quiz and Url shortener#4SwarnaLathaNatarajan wants to merge 6 commits intoSpectraLogic:masterfrom
Conversation
| key := entry["path"] | ||
| pathMap[key] = entry["url"] |
There was a problem hiding this comment.
It looks like you could have used a struct instead of a map here. Using a struct would be good practice for using go tags.
| return out, err | ||
| } | ||
|
|
||
| func buildMap(parsedYaml []map[string]string) map[string]string { |
There was a problem hiding this comment.
is this going to only accept YAML? The bonus objective is to also be able to use JSON.
| path, ok := pathsToUrls[r.URL.Path] | ||
| if ok { | ||
| http.Redirect(w, r, path, http.StatusFound) | ||
| } else { | ||
| fallback.ServeHTTP(w, r) | ||
| } |
There was a problem hiding this comment.
In go you will typically return instead of using else statements where appropriate.
| path, ok := pathsToUrls[r.URL.Path] | |
| if ok { | |
| http.Redirect(w, r, path, http.StatusFound) | |
| } else { | |
| fallback.ServeHTTP(w, r) | |
| } | |
| if path, ok := pathsToUrls[r.URL.Path]; ok { | |
| http.Redirect(w, r, path, http.StatusFound) | |
| } | |
| fallback.ServeHTTP(w, r) |
| return func(w http.ResponseWriter, r *http.Request) { | ||
| path, ok := pathsToUrls[r.URL.Path] | ||
| if ok { | ||
| http.Redirect(w, r, path, http.StatusFound) |
There was a problem hiding this comment.
Is http.StatusFound correct? I remember needing to use a 211 or some other status to signify a redirect.
There was a problem hiding this comment.
For the status code, I referred the following article https://stackoverflow.com/questions/22887148/is-the-http-statusfound-the-correct-status-for-a-click-redirect
|
|
||
| func parseYAML(yml []byte) (out []map[string]string, err error) { | ||
| err = yaml.Unmarshal(yml, &out) | ||
| return out, err |
There was a problem hiding this comment.
Using the named return values in the signature was a clever idea here. When you do that you don't need to specify the variables in the return statement (as long as you are returning the signature values).
| return MapHandler(pathMap, fallback), nil | ||
| } | ||
|
|
||
| func parseYAML(yml []byte) (out []map[string]string, err error) { |
There was a problem hiding this comment.
I'm not sure you need this function since it would only be two lines in the YAMLHandler function.
There was a problem hiding this comment.
The parseYAML stub was suggested in the problem. That's why I went ahead with it.
| @@ -33,5 +42,24 @@ func MapHandler(pathsToUrls map[string]string, fallback http.Handler) http.Handl | |||
| // a mapping of paths to urls. | |||
| func YAMLHandler(yml []byte, fallback http.Handler) (http.HandlerFunc, error) { | |||
| // TODO: Implement this... | |||
| @@ -12,7 +14,14 @@ import ( | |||
| // http.Handler will be called instead. | |||
| func MapHandler(pathsToUrls map[string]string, fallback http.Handler) http.HandlerFunc { | |||
| // TODO: Implement this... | |||
|
@gonzogomez I've updated the quiz problem with flags and pointer receivers as you mentioned. |
| } | ||
|
|
||
| func (q *Quiz) readProblems() { | ||
| csvfile, err := os.Open(q.filename) |
There was a problem hiding this comment.
You should probably make sure you close the file after opening it.
defer csvfile.Close()
| for i := 0; i < len(q.problems); i++ { | ||
| question, answer := q.problems[i][0], q.problems[i][1] | ||
| fmt.Println("What is " + question + " ?") | ||
| userAnswer, _ := in.ReadString('\n') |
There was a problem hiding this comment.
In Go, you'll typically see people iterating through slices using the range clause.
i.e., your code might look something like:
| for i := 0; i < len(q.problems); i++ { | |
| question, answer := q.problems[i][0], q.problems[i][1] | |
| fmt.Println("What is " + question + " ?") | |
| userAnswer, _ := in.ReadString('\n') | |
| for _, problem := range q.problems { | |
| question, answer := problem[0], problem[1] | |
| fmt.Printf("What is %s?\n", question) | |
| userAnswer, _ := in.ReadString('\n') | |
| // Rest of your code here | |
| } |
Also note the use of fmt.Printf() instead of fmt.Println(). It's not actually important in this case, given that question is just a string, but I thought I'd make a note just in case you weren't familiar.
There was a problem hiding this comment.
Oh okay. I'll make sure that I follow this in the future.
| filename string | ||
| timeLimit int | ||
| shuffle bool | ||
| problems [][]string |
There was a problem hiding this comment.
It seems like a Problem or Question struct would be better than a slice of slices.
| q.problems = records | ||
| } | ||
| func (q *Quiz) shuffleProblems() { | ||
| rand.Seed(time.Now().UnixNano()) |
There was a problem hiding this comment.
Seeding should only happen once and in the main function.
3 participants
Reviewers : @icubbon @JayDonigian @AdamO-SL @EmilyFOwens @gonzogomez