Add solution for Challenge 3 by clgp-aint-cool#1560
Add solution for Challenge 3 by clgp-aint-cool#1560github-actions[bot] merged 2 commits intoRezaSi:mainfrom
Conversation
WalkthroughA new Go solution file has been added that defines Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: a9613a49-83e4-43a7-87b7-dba7a68ec895
📒 Files selected for processing (1)
challenge-3/submissions/clgp-aint-cool/solution-template.go
| func (m *Manager) RemoveEmployee(id int) { | ||
| for i, e := range m.Employees { | ||
| if e.ID == id { | ||
| m.Employees = append(m.Employees[:i], m.Employees[i+1:]...) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Modifying slice while iterating can cause unexpected behavior.
After removing an element with append(m.Employees[:i], m.Employees[i+1:]...), the slice is shortened but the range loop continues. This can skip elements or cause index issues. Since employee IDs should be unique, add a return after removal to exit immediately.
🐛 Proposed fix
func (m *Manager) RemoveEmployee(id int) {
for i, e := range m.Employees {
if e.ID == id {
m.Employees = append(m.Employees[:i], m.Employees[i+1:]...)
+ return
}
}
}📝 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 (m *Manager) RemoveEmployee(id int) { | |
| for i, e := range m.Employees { | |
| if e.ID == id { | |
| m.Employees = append(m.Employees[:i], m.Employees[i+1:]...) | |
| } | |
| } | |
| } | |
| func (m *Manager) RemoveEmployee(id int) { | |
| for i, e := range m.Employees { | |
| if e.ID == id { | |
| m.Employees = append(m.Employees[:i], m.Employees[i+1:]...) | |
| return | |
| } | |
| } | |
| } |
|
🎉 Auto-merged! This PR was automatically merged after 2 days with all checks passing. Thank you for your contribution, @clgp-aint-cool! |
Challenge 3 Solution
Submitted by: @clgp-aint-cool
Challenge: Challenge 3
Description
This PR contains my solution for Challenge 3.
Changes
challenge-3/submissions/clgp-aint-cool/solution-template.goTesting
Thank you for reviewing my submission! 🚀