Add JSON concept exercise (marshal and unmarshal) - #88
Conversation
WalkthroughAdds a new JSON exercise (slug Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer/Test
participant EX as json package (exercise)
participant EJ as encoding/json
rect rgb(235,245,255)
note over Dev,EX: Marshal flow (new)
Dev->>EX: MarshalPerson(p)
EX->>EJ: json.Marshal(p)
EJ-->>EX: []byte / error
EX-->>Dev: string / error
end
rect rgb(240,255,240)
note over Dev,EX: Unmarshal flow (new)
Dev->>EX: UnmarshalPerson(jsonStr)
EX->>EJ: json.Unmarshal([]byte(jsonStr), &Person)
EJ-->>EX: error or Person
EX-->>Dev: Person / error
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
✨ Finishing touches🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
internal/exercises/templates/36_json/json_test.go (3)
13-15: Avoid brittle string-equality for JSON; compare semantically instead.String order/whitespace aren’t guaranteed across encoders. Prefer unmarshalling and comparing maps/structs.
Option A (compare as maps):
-import "testing" +import ( + "encoding/json" + "reflect" + "testing" +) @@ -expected := `{"name":"golearn","email":"golearn@example.com"}` -if jsonStr != expected { - t.Errorf("Expected %s, got %s", expected, jsonStr) -} +expected := `{"name":"golearn","email":"golearn@example.com"}` +var got, want map[string]any +if err := json.Unmarshal([]byte(jsonStr), &got); err != nil { + t.Fatalf("MarshalPerson returned invalid JSON: %v", err) +} +if err := json.Unmarshal([]byte(expected), &want); err != nil { + t.Fatalf("test setup invalid JSON: %v", err) +} +if !reflect.DeepEqual(got, want) { + t.Errorf("JSON mismatch.\nwant: %v\ngot: %v", want, got) +}Option B (keep string check but normalize): marshal both through json.Encoder with SetIndent/SetEscapeHTML and compare.
15-15: Quote values in failure messages for clarity.Using %q makes diffs clearer when strings contain spaces/special chars.
- t.Errorf("Expected %s, got %s", expected, jsonStr) + t.Errorf("Expected %q, got %q", expected, jsonStr) @@ - t.Errorf("Expected name 'golearn', got %s", p.Name) + t.Errorf("Expected name %q, got %q", "golearn", p.Name) @@ - t.Errorf("Expected email 'golearn@example.com', got %s", p.Email) + t.Errorf("Expected email %q, got %q", "golearn@example.com", p.Email)Also applies to: 27-31
19-33: Add a negative test to enforce error handling on invalid JSON.Hints mention proper error handling; a failing case will guard regressions.
func TestUnmarshalPerson(t *testing.T) { @@ } + +func TestUnmarshalPerson_InvalidJSON(t *testing.T) { + // missing closing brace + jsonStr := `{"name":"golearn","email":"golearn@example.com"` + _, err := UnmarshalPerson(jsonStr) + if err == nil { + t.Fatalf("Expected error for invalid JSON, got nil") + } +}internal/exercises/templates/36_json/json.go (2)
1-1: Package namejsoncan confuse learners alongside importingencoding/json.It’s valid, but referencing
json.Marshalwill point to the stdlib import while types here are unqualified. Consider adding a brief note in the task comment to clarify, or alias the import asstdjsonin examples/hints for readability.
16-19: Stubs return zero values with nil error; confirm CI excludes failing exercises.As written, tests will fail until learners implement these. Ensure your CI doesn’t run exercise tests (or marks them with build tags/skip) to avoid breaking main.
If you prefer to make intent explicit while still compiling, consider returning a sentinel error:
+import "errors" @@ -func MarshalPerson(p Person) (string, error) { - return "", nil -} +func MarshalPerson(p Person) (string, error) { + return "", errors.New("TODO: implement MarshalPerson") +} @@ -func UnmarshalPerson(jsonStr string) (Person, error) { - return Person{}, nil -} +func UnmarshalPerson(jsonStr string) (Person, error) { + return Person{}, errors.New("TODO: implement UnmarshalPerson") +}Alternatively, add a build tag (if that’s your convention) to keep exercise files out of CI. I can align this to your existing pattern if you share it.
Also applies to: 21-24
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
internal/exercises/catalog.yaml(1 hunks)internal/exercises/templates/36_json/json.go(1 hunks)internal/exercises/templates/36_json/json_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/exercises/templates/36_json/json_test.go (1)
internal/exercises/templates/36_json/json.go (3)
Person(11-14)MarshalPerson(17-19)UnmarshalPerson(22-24)
zhravan
added
patch
|
PR LGTM, thank you so much considering to contribute to the project |
2 participants
Summary
Added a new concept exercise for JSON under
internal/exercises/templates/36_json/.Details
json.goas a template file for learning JSON encoding and decoding.json_test.gowith tests forMarshalPersonandUnmarshalPerson.catalog.yamlto include the new JSON concept.encoding/jsonpackage in Go.Notes
All tests and formatting follow existing exercise conventions.
Closes #81
Summary by CodeRabbit