06 - Feat/formatting strings exercise - #97
Conversation
|
Warning Rate limit exceeded@kaushalyap has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 27 minutes and 48 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (1)
WalkthroughAdds six new concept exercises (defer, goroutines, channels, mutexes, sorting, string formatting) to the catalog and introduces matching template scaffolds, tests, and reference solutions. All changes add new files; no existing public APIs were removed or modified. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Test as Test (defer)
participant Tmpl as Template WriteToFile
participant OS as os.File
Test->>Tmpl: f := CreateFile()
Test->>Tmpl: WriteToFile(f)
activate Tmpl
Tmpl->>OS: write "data"
Tmpl-->>OS: defer Close()
deactivate Tmpl
Test->>OS: fmt.Fprintln(f, ...)
OS-->>Test: error (file closed)
sequenceDiagram
autonumber
actor Test as Test (goroutines)
participant Run as RunConcurrently
participant G1 as Goroutine A
participant G2 as Goroutine B
Test->>Run: invoke
par concurrent sleeps
Run->>G1: sleep 100ms
Run->>G2: sleep 200ms
end
Run-->>Test: return (< ~200ms total)
sequenceDiagram
autonumber
actor Test as Test (channels)
participant Read as ReadMessage
participant Sender as Goroutine (sender)
participant Ch as chan string
Test->>Read: call
Read->>Ch: make(chan string)
Read->>Sender: go func(){ Ch <- "Hi!" }
Sender-->>Ch: "Hi!"
Read-->>Test: <-Ch ("Hi!")
sequenceDiagram
autonumber
actor Test as Test (mutexes)
participant C as Counting
participant WG as WaitGroup
participant M as sync.Mutex
participant Done as done chan
Test->>C: call
loop N workers
C->>WG: Add(1)
C->>C: go worker
activate C
C->>M: Lock
C->>C: count++
C->>M: Unlock
C->>Done: signal
C->>WG: Done
deactivate C
end
C->>Done: wait N signals
C->>WG: Wait()
C-->>Test: return count
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
kawpii
changed the title
|
@zhravan I would appreciate if you could merge my already opened PRs, PRs are stacked one after other starting from 00 to 06 |
7f901e4 to
94fd4b3
Compare
|
Rebased and reformatted |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (3)
internal/exercises/templates/30_channels/channels_test.go (1)
5-11: Tighten the failure message formattingMinor nit: using
%qprints quoted strings and makes whitespace mismatches obvious. Consider:- t.Errorf("Expected %s, got %s", want, got) + t.Errorf("Expected %q, got %q", want, got)internal/exercises/solutions/29_go_routines/go_routines.go (1)
19-24: Remove or uncomment the alternative goroutine patterns.The commented code provides alternative goroutine invocation examples but adds clutter. Either uncomment it as additional documentation/examples (with explanatory comments), or remove it entirely if not needed.
internal/exercises/solutions/31_mutexes/mutexes.go (1)
9-30: Simplify by removing the redundant done channel.The
donechannel duplicates the synchronization already provided byWaitGroup. Sincewg.Wait()(line 29) already blocks until all goroutines complete, the done channel serves no additional purpose and adds unnecessary complexity.Apply this diff to simplify the synchronization:
func Counting() int { count := 0 - done := make(chan bool, numWorkers) var wg sync.WaitGroup var mu sync.Mutex for i := 1; i <= numWorkers; i++ { wg.Add(1) go func() { defer wg.Done() mu.Lock() count += 1 mu.Unlock() - done <- true }() } - for i := 1; i <= numWorkers; i++ { - <-done - } - wg.Wait() return count }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
internal/exercises/catalog.yaml(1 hunks)internal/exercises/solutions/28_defer/defer.go(1 hunks)internal/exercises/solutions/29_go_routines/go_routines.go(1 hunks)internal/exercises/solutions/30_channels/channels.go(1 hunks)internal/exercises/solutions/31_mutexes/mutexes.go(1 hunks)internal/exercises/solutions/32_sorting/sorting.go(1 hunks)internal/exercises/solutions/33_string_formatting/string_formatting.go(1 hunks)internal/exercises/templates/28_defer/defer.go(1 hunks)internal/exercises/templates/28_defer/defer_test.go(1 hunks)internal/exercises/templates/29_go_routines/go_routines.go(1 hunks)internal/exercises/templates/29_go_routines/go_routines_test.go(1 hunks)internal/exercises/templates/30_channels/channels.go(1 hunks)internal/exercises/templates/30_channels/channels_test.go(1 hunks)internal/exercises/templates/31_mutexes/mutexes.go(1 hunks)internal/exercises/templates/31_mutexes/mutexes_test.go(1 hunks)internal/exercises/templates/32_sorting/sorting.go(1 hunks)internal/exercises/templates/32_sorting/sorting_test.go(1 hunks)internal/exercises/templates/33_string_formatting/string_formatting.go(1 hunks)internal/exercises/templates/33_string_formatting/string_formatting_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-10-02T06:30:47.143Z
Learnt from: kaushalyap
PR: zhravan/golearn#110
File: internal/exercises/templates/28_defer/defer.go:14-19
Timestamp: 2025-10-02T06:30:47.143Z
Learning: In the defer exercise template (internal/exercises/templates/28_defer/defer.go), the unused *os.File parameter in WriteToFile is intentional and part of the pedagogical design for teaching defer concepts.
Applied to files:
internal/exercises/templates/28_defer/defer_test.gointernal/exercises/solutions/28_defer/defer.gointernal/exercises/templates/28_defer/defer.go
🧬 Code graph analysis (17)
internal/exercises/templates/29_go_routines/go_routines.go (1)
internal/exercises/solutions/29_go_routines/go_routines.go (1)
RunConcurrently(15-25)
internal/exercises/templates/28_defer/defer_test.go (2)
internal/exercises/solutions/28_defer/defer.go (2)
CreateFile(14-23)WriteToFile(25-29)internal/exercises/templates/28_defer/defer.go (2)
CreateFile(14-23)WriteToFile(25-29)
internal/exercises/solutions/33_string_formatting/string_formatting.go (1)
internal/exercises/templates/33_string_formatting/string_formatting.go (3)
FormatName(5-8)FormatAge(10-13)FormatGpa(15-18)
internal/exercises/templates/32_sorting/sorting.go (1)
internal/exercises/solutions/32_sorting/sorting.go (4)
SortNumbers(10-13)Years(7-7)SortAnimals(15-18)Pets(8-8)
internal/exercises/templates/33_string_formatting/string_formatting.go (1)
internal/exercises/solutions/33_string_formatting/string_formatting.go (3)
FormatName(5-7)FormatAge(9-11)FormatGpa(13-15)
internal/exercises/templates/31_mutexes/mutexes_test.go (2)
internal/exercises/solutions/31_mutexes/mutexes.go (1)
Counting(7-31)internal/exercises/templates/31_mutexes/mutexes.go (1)
Counting(7-30)
internal/exercises/templates/30_channels/channels.go (1)
internal/exercises/solutions/30_channels/channels.go (1)
ReadMessage(3-13)
internal/exercises/solutions/29_go_routines/go_routines.go (1)
internal/exercises/templates/29_go_routines/go_routines.go (1)
RunConcurrently(15-19)
internal/exercises/templates/31_mutexes/mutexes.go (1)
internal/exercises/solutions/31_mutexes/mutexes.go (1)
Counting(7-31)
internal/exercises/solutions/30_channels/channels.go (1)
internal/exercises/templates/30_channels/channels.go (1)
ReadMessage(5-15)
internal/exercises/templates/29_go_routines/go_routines_test.go (2)
internal/exercises/solutions/29_go_routines/go_routines.go (1)
RunConcurrently(15-25)internal/exercises/templates/29_go_routines/go_routines.go (1)
RunConcurrently(15-19)
internal/exercises/templates/32_sorting/sorting_test.go (1)
internal/exercises/templates/32_sorting/sorting.go (4)
SortNumbers(6-9)Years(3-3)SortAnimals(11-14)Pets(4-4)
internal/exercises/solutions/31_mutexes/mutexes.go (1)
internal/exercises/templates/31_mutexes/mutexes.go (1)
Counting(7-30)
internal/exercises/templates/30_channels/channels_test.go (2)
internal/exercises/solutions/30_channels/channels.go (1)
ReadMessage(3-13)internal/exercises/templates/30_channels/channels.go (1)
ReadMessage(5-15)
internal/exercises/templates/33_string_formatting/string_formatting_test.go (2)
internal/exercises/solutions/33_string_formatting/string_formatting.go (3)
FormatName(5-7)FormatAge(9-11)FormatGpa(13-15)internal/exercises/templates/33_string_formatting/string_formatting.go (3)
FormatName(5-8)FormatAge(10-13)FormatGpa(15-18)
internal/exercises/templates/28_defer/defer.go (1)
internal/exercises/solutions/28_defer/defer.go (2)
CreateFile(14-23)WriteToFile(25-29)
internal/exercises/solutions/32_sorting/sorting.go (1)
internal/exercises/templates/32_sorting/sorting.go (4)
SortNumbers(6-9)Years(3-3)SortAnimals(11-14)Pets(4-4)
🪛 GitHub Actions: CI
internal/exercises/templates/33_string_formatting/string_formatting.go
[error] 7-7: go vet: fmt.Sprintf call has arguments but no formatting directives.
🪛 GitHub Check: test
internal/exercises/templates/33_string_formatting/string_formatting.go
[failure] 17-17:
fmt.Sprintf call has arguments but no formatting directives
[failure] 12-12:
fmt.Sprintf call has arguments but no formatting directives
[failure] 7-7:
fmt.Sprintf call has arguments but no formatting directives
🔇 Additional comments (16)
internal/exercises/solutions/30_channels/channels.go (1)
3-12: Solid channel hand-offNice, minimal illustration of using an unbuffered channel to shuttle data back to the caller; this will mirror the learner-facing exercise cleanly.
internal/exercises/solutions/28_defer/defer.go (1)
14-29: Defer usage is on pointThe once-guarded factory plus deferred close cleanly demonstrates the intended pattern for learners. Looks good.
internal/exercises/templates/30_channels/channels.go (1)
5-15: Clear learner guidanceAppreciate the explicit TODO explaining the expected channel flow—concise scaffolding while still leaving the work to the student.
internal/exercises/templates/28_defer/defer.go (1)
14-29: Template matches solution expectationsThe scaffold mirrors the solution closely and the TODO clearly marks where learners should introduce defer logic—nicely aligned with the exercise goals.
internal/exercises/templates/28_defer/defer_test.go (1)
8-17: LGTM! Test correctly validates defer-based file closing.The test appropriately verifies that the file is closed after
WriteToFilereturns by attempting a subsequent write and expecting an error. This aligns with the exercise's pedagogical goal of teaching defer-based resource management.internal/exercises/templates/32_sorting/sorting.go (1)
1-14: LGTM! Template correctly scaffolds the sorting exercise.The TODO comments and placeholder implementations provide clear guidance for students to implement sorting using
slices.Sort. The structure aligns with the corresponding solution and test expectations.internal/exercises/templates/32_sorting/sorting_test.go (1)
16-21: LGTM! String sorting test is correct.The subtest correctly validates that
Petsis sorted after callingSortAnimals().internal/exercises/solutions/32_sorting/sorting.go (1)
1-18: LGTM! Solution correctly implements in-place sorting.Both
SortNumbersandSortAnimalsappropriately useslices.Sortto sort the public slices in-place and return them, demonstrating the intended solution pattern for the exercise.internal/exercises/templates/31_mutexes/mutexes_test.go (1)
5-12: LGTM! Test correctly validates concurrent counting.The test appropriately checks that
Counting()returnsnumWorkers, which will validate that the student has correctly implemented mutex-based synchronization to prevent race conditions in the template exercise.internal/exercises/templates/33_string_formatting/string_formatting_test.go (1)
7-34: LGTM! String formatting tests are correctly structured.The three subtests appropriately validate the expected output formats for name (string with quotes), age (integer), and GPA (float with two decimals). The assertions align with the solution implementations.
internal/exercises/catalog.yaml (2)
137-156: LGTM! Defer, go_routines, channels, and mutexes entries are well-structured.The catalog entries for concepts 28–31 follow the established format and provide helpful hints that align with their respective implementations and tests.
163-166: LGTM! String formatting entry is correct.The catalog entry for concept 33 appropriately describes string formatting with clear hints for
%s,%d, and%fformat verbs.internal/exercises/templates/29_go_routines/go_routines.go (2)
7-13: LGTM!The sleep helper functions are correctly implemented with appropriate durations for the concurrency exercise.
15-19: LGTM!The template provides appropriate scaffolding for learners. The TODO comment clearly indicates where learners should implement concurrent execution using goroutines.
internal/exercises/templates/31_mutexes/mutexes.go (1)
7-30: LGTM!The template provides appropriate scaffolding with a clear TODO indicating where learners should add mutex synchronization to prevent the race condition on line 19.
internal/exercises/solutions/33_string_formatting/string_formatting.go (1)
5-15: LGTM!All three formatting functions correctly use appropriate format verbs:
%sfor string formatting%dfor integer formatting%.2ffor floating-point formatting with two decimal placesThe implementations are correct and match the expected test outputs.
| - slug: 32_sorting | ||
| title: Sorting | ||
| test_regex: ".*" | ||
| hints: | ||
| - Use `slice.Sort` for sorting slices. | ||
| - slug: 33_string_formatting |
There was a problem hiding this comment.
Fix the package name in the sorting hint.
Line 161 references slice.Sort, but the correct package for the sorting function is slices.Sort (available in Go 1.21+). This should be corrected to match the actual implementation in the solution files.
Apply this diff:
hints:
- - Use `slice.Sort` for sorting slices.
+ - Use `slices.Sort` for sorting slices.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - slug: 32_sorting | |
| title: Sorting | |
| test_regex: ".*" | |
| hints: | |
| - Use `slice.Sort` for sorting slices. | |
| - slug: 33_string_formatting | |
| - slug: 32_sorting | |
| title: Sorting | |
| test_regex: ".*" | |
| hints: | |
| - Use `slices.Sort` for sorting slices. | |
| - slug: 33_string_formatting |
🤖 Prompt for AI Agents
In internal/exercises/catalog.yaml around lines 157 to 162, update the hint that
currently references the wrong package name; replace the incorrect `slice.Sort`
with the correct `slices.Sort` (Go 1.21+) so the hint matches the implementation
in the solution files and use backticks around the symbol if the file format
expects code formatting.
| func TestSorting(t *testing.T) { | ||
| t.Run("Sorting numbers", func(t *testing.T) { | ||
| SortNumbers() | ||
| if !slices.IsSorted(Years) { | ||
| t.Fatal("Strings are not sorted!") | ||
| } | ||
| }) |
There was a problem hiding this comment.
Fix the error message to match the test context.
Line 12 reports "Strings are not sorted!" but this subtest is validating number sorting. The message should say "Numbers are not sorted!" for clarity.
Apply this diff:
if !slices.IsSorted(Years) {
- t.Fatal("Strings are not sorted!")
+ t.Fatal("Numbers are not sorted!")
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| func TestSorting(t *testing.T) { | |
| t.Run("Sorting numbers", func(t *testing.T) { | |
| SortNumbers() | |
| if !slices.IsSorted(Years) { | |
| t.Fatal("Strings are not sorted!") | |
| } | |
| }) | |
| func TestSorting(t *testing.T) { | |
| t.Run("Sorting numbers", func(t *testing.T) { | |
| SortNumbers() | |
| if !slices.IsSorted(Years) { | |
| t.Fatal("Numbers are not sorted!") | |
| } | |
| }) |
🤖 Prompt for AI Agents
In internal/exercises/templates/32_sorting/sorting_test.go around lines 8 to 14,
the test's failure message is incorrect for the subtest; change the t.Fatal
message from "Strings are not sorted!" to "Numbers are not sorted!" so the error
text matches the "Sorting numbers" subtest context.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/exercises/catalog.yaml (1)
157-161: Fix the package name in the sorting hint.Line 161 still references
slice.Sort, but the correct package for the sorting function isslices.Sort(available in Go 1.21+). This issue was previously flagged but remains unresolved.Apply this diff:
hints: - - Use `slice.Sort` for sorting slices. + - Use `slices.Sort` for sorting slices.
🧹 Nitpick comments (1)
internal/exercises/templates/33_string_formatting/string_formatting.go (1)
3-8: Consider clarifying the expected output format.The template structure is good for a learning exercise. However, line 6 states
Name value is \"John\"which might be ambiguous. Based on the expected test outputName: "John", learners need to include the quotes in the output string.Consider this clearer wording:
// TODO: format the name as a string // Follow the format of "Name: John" - // Name value is \"John\" + // The name should be formatted with quotes: "John" return ""
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/exercises/catalog.yaml(1 hunks)internal/exercises/templates/33_string_formatting/string_formatting.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
internal/exercises/templates/33_string_formatting/string_formatting.go (1)
internal/exercises/solutions/33_string_formatting/string_formatting.go (3)
FormatName(5-7)FormatAge(9-11)FormatGpa(13-15)
🔇 Additional comments (6)
internal/exercises/catalog.yaml (4)
142-146: LGTM!The go routines hint accurately describes using the
gokeyword for concurrent execution.
147-151: LGTM!The channels hint correctly describes channel creation and the send/receive operator syntax.
152-156: LGTM!The mutexes hint correctly references
sync.Mutexand its purpose for synchronizing access to shared resources.
162-166: LGTM!The string formatting hint accurately describes the use of
fmt.Sprintfwith the essential format directives for strings, integers, and floats. This aligns well with the PR objectives.internal/exercises/templates/33_string_formatting/string_formatting.go (2)
10-15: LGTM!The template provides clear guidance for formatting an integer. The instructions are straightforward and appropriate for learners.
17-22: LGTM!The template provides excellent guidance for floating-point formatting, explicitly mentioning the 2 decimal places requirement. This helps learners understand precision control in format strings.
| - slug: 28_defer | ||
| title: Defer | ||
| test_regex: ".*" | ||
| hints: | ||
| - Use `f.close()` with `defer` keyword. |
There was a problem hiding this comment.
Fix the method name in the defer hint.
Line 141 references f.close(), but in Go, the correct method is f.Close() (capitalized). File close methods follow Go's exported identifier conventions.
Apply this diff:
hints:
- - Use `f.close()` with `defer` keyword.
+ - Use `f.Close()` with `defer` keyword.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - slug: 28_defer | |
| title: Defer | |
| test_regex: ".*" | |
| hints: | |
| - Use `f.close()` with `defer` keyword. | |
| - slug: 28_defer | |
| title: Defer | |
| test_regex: ".*" | |
| hints: | |
| - Use `f.Close()` with `defer` keyword. |
🤖 Prompt for AI Agents
In internal/exercises/catalog.yaml around lines 137 to 141, the hint uses the
incorrect Go method name `f.close()`; update the hint to use the exported Go
method `f.Close()` (capital C) so the guidance matches Go's identifier
conventions and will compile when referenced.
1 participant
Summary
Adds string formatting exercise. Includes exercises to format double-quoted strings, integers, floating point numbers since they are the most common cases. Value formatting (structs, slices) can be added later on.
Checklist
make verifyorgolearn verify <slug>(CLI does not work as said in an issue, but tested locally usinggo test.)Screenshots / Output (if CLI UX)
Paste before/after where helpful.
Related issues
Fixes #78
Summary by CodeRabbit
New Features
Tests
Chores