Add solution for Challenge 3 by Sairaviteja27 #611
Conversation
WalkthroughIntroduces a Go program implementing employee management functionality with an Employee struct containing ID, Name, Age, and Salary fields, and a Manager struct with methods to add/remove employees, calculate average salary, and search for employees by ID. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes The change introduces straightforward data structures and slice-based CRUD operations. Review should focus on validating the slice manipulation logic in RemoveEmployee, verifying the average salary calculation handles edge cases correctly, and identifying the noted pointer-to-loop-variable issue in FindEmployeeByID that could produce invalid memory references after iteration completes. Possibly related PRs
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
![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
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-3/submissions/Sairaviteja27/solution-template.go(1 hunks)
🔇 Additional comments (4)
challenge-3/submissions/Sairaviteja27/solution-template.go (4)
16-19: LGTM!The method correctly appends the employee to the manager's list using the pointer receiver.
21-29: LGTM!The method correctly removes the first employee with the matching ID using slice manipulation and breaks appropriately.
31-43: LGTM!The method correctly calculates the average salary with proper handling of the empty slice case.
55-67: Good demonstration of the API.The main function correctly demonstrates all the manager methods with proper nil checking before dereferencing the employee pointer.
| // FindEmployeeByID finds and returns an employee by their ID. | ||
| func (m *Manager) FindEmployeeByID(id int) *Employee { | ||
| for _,v:= range m.Employees { | ||
| if v.ID==id{ | ||
| return &v | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Critical bug: returning pointer to loop variable.
Line 49 returns &v, which is the address of the loop variable. In Go, the loop variable is reused for each iteration, so this returns an invalid pointer that will not correctly reference the employee in the slice.
Apply this diff to fix the issue by returning a pointer to the slice element:
- for _,v:= range m.Employees {
- if v.ID==id{
- return &v
+ for i, v := range m.Employees {
+ if v.ID == id {
+ return &m.Employees[i]
}
}Alternatively, you can iterate by index only:
- for _,v:= range m.Employees {
- if v.ID==id{
- return &v
+ for i := range m.Employees {
+ if m.Employees[i].ID == id {
+ return &m.Employees[i]
}
}📝 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.
| // FindEmployeeByID finds and returns an employee by their ID. | |
| func (m *Manager) FindEmployeeByID(id int) *Employee { | |
| for _,v:= range m.Employees { | |
| if v.ID==id{ | |
| return &v | |
| } | |
| } | |
| return nil | |
| } | |
| // FindEmployeeByID finds and returns an employee by their ID. | |
| func (m *Manager) FindEmployeeByID(id int) *Employee { | |
| for i, v := range m.Employees { | |
| if v.ID == id { | |
| return &m.Employees[i] | |
| } | |
| } | |
| return nil | |
| } |
🤖 Prompt for AI Agents
In challenge-3/submissions/Sairaviteja27/solution-template.go around lines 45 to
53, the function returns &v which is the address of the reused loop variable;
change the loop to either iterate by index or return the address of the actual
slice element (e.g., for i := range m.Employees { if m.Employees[i].ID == id {
return &m.Employees[i] } }) so you return a stable pointer to the element; keep
the final return nil unchanged.
2 participants
Challenge 3 Solution
Submitted by: @Sairaviteja27
Challenge: Challenge 3
Description
This PR contains my solution for Challenge 3.
Changes
challenge-3/submissions/Sairaviteja27/solution-template.goTesting
Thank you for reviewing my submission! 🚀