Add solution for Challenge 3 by baindraraj #613
Conversation
WalkthroughA new Go program is introduced that implements an Employee struct and a Manager struct with methods to add employees, remove employees by ID, calculate average salary, and find employees by ID, including a main function demonstrating these operations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes The changes introduce straightforward Go types and basic methods with simple logic (appending, iteration, filtering). The code is self-contained in a single file with no complex control flow or business logic. 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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-3/submissions/baindraraj/solution-template.go(1 hunks)
🔇 Additional comments (3)
challenge-3/submissions/baindraraj/solution-template.go (3)
5-10: LGTM!The Employee struct is well-defined with appropriate field types and exported fields.
12-14: LGTM!The Manager struct is appropriately defined to hold a collection of employees.
61-73: LGTM with dependency on bug fix.The main function appropriately demonstrates the usage of the Manager and its methods. Once the critical bug in
FindEmployeeByIDis fixed, this demonstration will work correctly.
| // AddEmployee adds a new employee to the manager's list. | ||
| func (m *Manager) AddEmployee(e Employee) { | ||
| // TODO: Implement this method | ||
| m.Employees = append(m.Employees, e) | ||
| } |
There was a problem hiding this comment.
Remove TODO comment.
The method is implemented correctly, but the TODO comment on Line 18 should be removed.
- // TODO: Implement this method
m.Employees = append(m.Employees, e)📝 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.
| // AddEmployee adds a new employee to the manager's list. | |
| func (m *Manager) AddEmployee(e Employee) { | |
| // TODO: Implement this method | |
| m.Employees = append(m.Employees, e) | |
| } | |
| // AddEmployee adds a new employee to the manager's list. | |
| func (m *Manager) AddEmployee(e Employee) { | |
| m.Employees = append(m.Employees, e) | |
| } |
🤖 Prompt for AI Agents
In challenge-3/submissions/baindraraj/solution-template.go around lines 16 to
20, remove the leftover TODO comment on line 18 since the AddEmployee method is
implemented correctly; simply delete the "// TODO: Implement this method" line
so the function contains only the implementation and any needed comments
describing behavior.
| // RemoveEmployee removes an employee by ID from the manager's list. | ||
| func (m *Manager) RemoveEmployee(id int) { | ||
| // TODO: Implement this method | ||
| for indx, e := range m.Employees { | ||
| if e.ID == id { | ||
| m.Employees = append(m.Employees[:indx], m.Employees[indx+1:]...) | ||
| break | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Remove TODO comment.
The method is implemented correctly, but the TODO comment on Line 24 should be removed.
- // TODO: Implement this method
for indx, e := range m.Employees {📝 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.
| // RemoveEmployee removes an employee by ID from the manager's list. | |
| func (m *Manager) RemoveEmployee(id int) { | |
| // TODO: Implement this method | |
| for indx, e := range m.Employees { | |
| if e.ID == id { | |
| m.Employees = append(m.Employees[:indx], m.Employees[indx+1:]...) | |
| break | |
| } | |
| } | |
| } | |
| // RemoveEmployee removes an employee by ID from the manager's list. | |
| func (m *Manager) RemoveEmployee(id int) { | |
| for indx, e := range m.Employees { | |
| if e.ID == id { | |
| m.Employees = append(m.Employees[:indx], m.Employees[indx+1:]...) | |
| break | |
| } | |
| } | |
| } |
🤖 Prompt for AI Agents
In challenge-3/submissions/baindraraj/solution-template.go around lines 22 to
31, remove the TODO comment on line 24 because the RemoveEmployee method is
already implemented; delete the "// TODO: Implement this method" line so the
comment no longer incorrectly indicates incomplete work and leave the
implemented loop and logic intact.
| // GetAverageSalary calculates the average salary of all employees. | ||
| func (m *Manager) GetAverageSalary() float64 { | ||
| // TODO: Implement this method | ||
| if len(m.Employees) == 0 { | ||
| return 0 | ||
| } | ||
|
|
||
| var total float64 | ||
| cnt := 0 | ||
| for _, e := range m.Employees { | ||
| total += e.Salary | ||
| cnt++ | ||
| } | ||
|
|
||
| return total/ float64(cnt) | ||
| } |
There was a problem hiding this comment.
Remove TODO comment and simplify logic.
The method works correctly but has a few minor issues:
- Line 35: TODO comment should be removed
- Line 47: Missing space before
/operator - Lines 41-44: The
cntvariable is redundant sincelen(m.Employees)is already known
- // TODO: Implement this method
if len(m.Employees) == 0 {
return 0
}
var total float64
- cnt := 0
for _, e := range m.Employees {
total += e.Salary
- cnt++
}
- return total/ float64(cnt)
+ return total / float64(len(m.Employees))📝 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.
| // GetAverageSalary calculates the average salary of all employees. | |
| func (m *Manager) GetAverageSalary() float64 { | |
| // TODO: Implement this method | |
| if len(m.Employees) == 0 { | |
| return 0 | |
| } | |
| var total float64 | |
| cnt := 0 | |
| for _, e := range m.Employees { | |
| total += e.Salary | |
| cnt++ | |
| } | |
| return total/ float64(cnt) | |
| } | |
| // GetAverageSalary calculates the average salary of all employees. | |
| func (m *Manager) GetAverageSalary() float64 { | |
| if len(m.Employees) == 0 { | |
| return 0 | |
| } | |
| var total float64 | |
| for _, e := range m.Employees { | |
| total += e.Salary | |
| } | |
| return total / float64(len(m.Employees)) | |
| } |
🤖 Prompt for AI Agents
In challenge-3/submissions/baindraraj/solution-template.go around lines 33 to
48, remove the TODO comment, eliminate the redundant cnt variable and use
len(m.Employees) directly, and fix spacing by adding a space before the division
operator so the return becomes total / float64(len(m.Employees)); keep the
empty-slice check returning 0 as-is.
| // FindEmployeeByID finds and returns an employee by their ID. | ||
| func (m *Manager) FindEmployeeByID(id int) *Employee { | ||
| // TODO: Implement this method | ||
| for _, e := range m.Employees { | ||
| if e.ID == id { | ||
| return &e | ||
| } | ||
| } | ||
| return nil | ||
| } |
There was a problem hiding this comment.
Critical bug: returning address of loop variable.
Line 55 returns &e, where e is the range loop variable. In Go, the loop variable is reused in each iteration, so its address remains constant. This means &e will point to the memory location of the loop variable, not the actual employee in the slice. After the loop, this address may contain the last iterated value or be in an undefined state.
- // TODO: Implement this method
- for _, e := range m.Employees {
+ for i := range m.Employees {
+ e := &m.Employees[i]
if e.ID == id {
- return &e
+ return e
}
}
return nil🤖 Prompt for AI Agents
In challenge-3/submissions/baindraraj/solution-template.go around lines 50 to
59, the method currently returns &e where e is the range loop variable — fix by
returning the address of the actual slice element instead of the loop variable;
iterate with index (for i := range m.Employees) or use for i, e := range
m.Employees and return &m.Employees[i] when IDs match so the pointer refers to
the real Employee in the slice.
2 participants
Challenge 3 Solution
Submitted by: @baindraraj
Challenge: Challenge 3
Description
This PR contains my solution for Challenge 3.
Changes
challenge-3/submissions/baindraraj/solution-template.goTesting
Thank you for reviewing my submission! 🚀