-
Notifications
You must be signed in to change notification settings - Fork 24
Feat/stateful goroutines exercise #112
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Feat/stateful goroutines exercise #112
Conversation
This commit implements issue zhravan#71: Add exercise templates and tests for Stateful Goroutines concept. Changes: - Added exercise template at internal/exercises/templates/28_stateful_goroutines/ - stateful_goroutines.go: Incomplete template with TODO comments demonstrating the stateful goroutines pattern using channels for state management - stateful_goroutines_test.go: Comprehensive test suite covering: * Counter initialization * Basic increment operations * Concurrent increments from multiple goroutines * Concurrent reads and writes * Negative increment support - Added working solution at internal/exercises/solutions/28_stateful_goroutines/ - Implements the channel-based state management pattern - Uses a single goroutine to own state (avoiding race conditions) - readOp and writeOp structs for communication via channels - select statement for handling concurrent operations - Updated internal/exercises/catalog.yaml - Added slug: 28_stateful_goroutines - Title: Stateful Goroutines - Comprehensive hints explaining the pattern: * Channel-based communication * readOp and writeOp structs with response channels * State-owning goroutine with select * Avoiding mutexes through single-goroutine ownership Testing: - Template tests fail as expected (4/5 tests fail due to incomplete implementation) - Solution passes all tests (5/5 tests pass) - Tests validate concurrent safety and correctness This exercise teaches the Go concurrency pattern of using goroutines and channels to manage shared state, aligning with Go's principle of 'share memory by communicating' rather than using explicit locks.
- Fix whitespace formatting in solution file (remove trailing spaces) - Update .gitignore to exclude Codacy artifacts: - .codacy/ directory - .github/instructions/ directory These files are tool-generated and should not be committed to the repository.
WalkthroughAdds a new “Stateful Goroutines” exercise: catalog entry with hints, a template and tests, and a complete solution implementing a counter via a single state-owning goroutine and channels. Updates .gitignore to adjust bin path handling and ignore Codacy and GitHub instructions folders. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant Counter
participant Goroutine as State Goroutine
Note over Counter,Goroutine: Initialization
Client->>Counter: NewCounter()
Counter->>Goroutine: start loop (state := 0)
rect rgba(220,240,255,0.5)
Note over Client,Goroutine: Write operation
Client->>Counter: Increment(amount)
Counter->>Goroutine: writeOp{amount, resp}
Goroutine->>Goroutine: state += amount
Goroutine-->>Counter: resp <- true
Counter-->>Client: return
end
rect rgba(220,255,220,0.5)
Note over Client,Goroutine: Read operation
Client->>Counter: GetValue()
Counter->>Goroutine: readOp{resp}
Goroutine-->>Counter: resp <- state
Counter-->>Client: return value
end
rect rgba(255,235,220,0.5)
Note over Client,Goroutine: Shutdown
Client->>Counter: Close()
Counter->>Goroutine: done closed
Goroutine->>Goroutine: exit loop
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
Run 'go fmt ./...' to fix formatting issues in test file. This resolves the CI/CD pipeline failure caused by improper code formatting. Fixed file: - internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (corrected whitespace formatting)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🧹 Nitpick comments (2)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (1)
62-101: Consider removing time.Sleep calls for faster, equally effective tests.The
time.Sleep(time.Microsecond)calls at lines 76 and 88 attempt to interleave operations but don't provide stronger guarantees than the Go scheduler already provides. Removing them would make tests faster without reducing coverage.Apply this diff to remove the sleeps:
defer wg.Done() for j := 0; j < 5; j++ { counter.Increment(1) - time.Sleep(time.Microsecond) } }() } // Start readers for i := 0; i < numReaders; i++ { wg.Add(1) go func() { defer wg.Done() for j := 0; j < 5; j++ { _ = counter.GetValue() - time.Sleep(time.Microsecond) } }() }If you prefer to keep them, the test remains correct.
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
19-60: Consider timeout handling for production use.While the current implementation correctly demonstrates the stateful goroutine pattern, production code should consider adding timeout mechanisms to prevent indefinite blocking if the state goroutine becomes unresponsive.
Example approach using context:
func (c *Counter) GetValueWithContext(ctx context.Context) (int, error) { read := readOp{resp: make(chan int)} select { case c.reads <- read: select { case value := <-read.resp: return value, nil case <-ctx.Done(): return 0, ctx.Err() } case <-ctx.Done(): return 0, ctx.Err() } }This is optional for the teaching exercise but valuable for real-world use.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.gitignore(1 hunks)internal/exercises/catalog.yaml(1 hunks)internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go(1 hunks)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go(1 hunks)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (2)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
NewCounter(20-41)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (1)
NewCounter(26-29)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (2)
NewCounter(26-29)Counter(20-23)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (1)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (2)
NewCounter(20-41)Counter(14-17)
🔇 Additional comments (12)
.gitignore (1)
33-37: Ignore rules look good.Switching to
bin/properly catches build artifacts, and adding the Codacy/GitHub instruction directories keeps generated/config outputs out of version control. ✔️internal/exercises/catalog.yaml (1)
137-144: LGTM! Clear and well-structured catalog entry.The hints provide a logical progression explaining the channel-based state management pattern, from operation channels through to the rationale for avoiding mutexes.
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (3)
24-34: LGTM! Clean sequential increment test.The test correctly validates basic increment functionality without introducing artificial delays or complexity.
36-60: LGTM! Thorough concurrent increment test.The test validates the core concurrency safety guarantee of the pattern with 100 goroutines performing 1,000 total increments. The use of
sync.WaitGroupensures proper synchronization.
103-113: LGTM! Validates negative increment handling.The test correctly verifies that negative amounts work as expected (effectively decrementing the counter).
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines.go (4)
1-8: LGTM! Clear introduction and requirements.The TODO comment effectively sets expectations and explains the core concept of single-goroutine state ownership with channel-based communication.
9-23: LGTM! Type definitions provide necessary structure.The complete type definitions (readOp, writeOp, Counter) give learners the concrete scaffolding they need to implement the channel-based state management pattern, while leaving the core logic as exercises.
25-29: Template stub is intentionally incomplete (as expected).Returning an uninitialized
Counter{}with nil channels will cause panics when tests callIncrementorGetValue, which is expected behavior for an incomplete template—learners must initialize the channels and start the goroutine.
31-40: LGTM! Clear method stubs with helpful TODOs.The TODO comments guide learners to implement the channel send/receive pattern for writes (with confirmation) and reads (with value return). The stubs are appropriately incomplete for a teaching template.
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (3)
3-17: Type definitions match template correctly.The types align with the template scaffolding, ensuring consistency between the learning exercise and the reference solution.
43-51: LGTM! Correct synchronous write implementation.The method correctly creates a one-time response channel, sends the write operation, and blocks until acknowledged. This ensures the increment completes before returning.
53-60: LGTM! Correct synchronous read implementation.The method correctly creates a one-time response channel, sends the read operation, and blocks until the state value is returned. The pattern mirrors
Incrementappropriately.
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go
Show resolved
Hide resolved
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go
Outdated
Show resolved
Hide resolved
- Remove unnecessary time.Sleep from TestCounterInitialization test - Add Close() method to Counter to prevent goroutine leaks - Add done channel to properly terminate state-owning goroutine Fixes review comments from zhravan#112 (review)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (1)
59-98: Consider removing the time.Sleep calls for faster, more robust tests.The
time.Sleep(time.Microsecond)calls at lines 73 and 85 artificially slow down concurrent operations. The channel-based implementation should naturally handle rapid concurrent reads and writes without artificial delays. Removing these sleeps will make the test faster and eliminate potential timing-dependent behavior.Apply this diff to remove the sleeps:
wg.Add(1) go func() { defer wg.Done() for j := 0; j < 5; j++ { counter.Increment(1) - time.Sleep(time.Microsecond) } }() } // Start readers for i := 0; i < numReaders; i++ { wg.Add(1) go func() { defer wg.Done() for j := 0; j < 5; j++ { _ = counter.GetValue() - time.Sleep(time.Microsecond) } }() }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go(1 hunks)internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go
🧰 Additional context used
🧬 Code graph analysis (1)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (1)
internal/exercises/solutions/28_stateful_goroutines/stateful_goroutines.go (1)
NewCounter(21-45)
🔇 Additional comments (4)
internal/exercises/templates/28_stateful_goroutines/stateful_goroutines_test.go (4)
9-19: LGTM!The initialization test correctly validates that
NewCounter()returns a non-nil counter and that the initial value is 0. The test is straightforward and correct.
21-31: LGTM!The test correctly validates basic increment functionality in a single-threaded context. The logic is sound: two increments (5 and 3) should result in a total of 8.
33-57: LGTM!The concurrent increments test properly validates thread-safety with 100 goroutines performing 10 increments each. The use of
WaitGroupensures all operations complete before the final assertion, and the expected value calculation (1000) is correct.
100-110: LGTM!The test correctly validates that
Incrementhandles negative deltas. The logic is sound: incrementing by 10 then by -3 should result in a final value of 7.
|
@aviralgarg05: Can you change and resolve current conflicts? |
Summary
Feat/stateful goroutines exercise
Changes:
Added exercise template at internal/exercises/templates/28_stateful_goroutines/
the stateful goroutines pattern using channels for state management
Added working solution at internal/exercises/solutions/28_stateful_goroutines/
Updated internal/exercises/catalog.yaml
Testing:
This exercise teaches the Go concurrency pattern of using goroutines and channels
to manage shared state, aligning with Go's principle of 'share memory by communicating'
rather than using explicit locks.
Checklist
make verifyorgolearn verify <slug>Screenshots / Output (if CLI UX)
Paste before/after where helpful.
Related issues
Fixes #71
For Hacktoberfest 2025
Summary by CodeRabbit