-
-
Notifications
You must be signed in to change notification settings - Fork 709
Add solution for Challenge 3 by AlexO-85 #690
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
Conversation
WalkthroughA new Go solution file is introduced that defines an Employee type with ID, Name, Age, and Salary fields, along with a Manager type containing a slice of employees. Four manager methods implement employee lifecycle operations: adding, removing, average salary calculation, and ID-based lookup. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
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 |
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 (3)
challenge-3/submissions/AlexO-85/solution-template.go (3)
21-29: RemoveEmployee logic is correct.The slice manipulation correctly removes the first matching employee. Consider these optional style improvements:
- Line 24: Parentheses around
ifconditions are not idiomatic in Go.- Line 26: The semicolon after
breakis unnecessary.Apply this diff for Go style conventions:
- if(e.ID == id){ + if e.ID == id { m.Employees = append(m.Employees[:i], m.Employees[i+1:]...) - break; + break }
31-48: GetAverageSalary correctly handles edge cases.The method properly guards against division by zero. Consider these optional refinements:
- Line 37: Parentheses around
ifconditions are not idiomatic in Go.- Line 45: Reuse
countEmployeesinstead of callinglen()again for consistency.Apply this diff for minor improvements:
- if(countEmployees == 0){ + if countEmployees == 0 { return sumSalary } for _, e := range m.Employees { sumSalary += e.Salary } - avgSalary := sumSalary / float64(len(m.Employees)) + avgSalary := sumSalary / float64(countEmployees) return avgSalary
50-58: FindEmployeeByID correctly returns a pointer.The method appropriately returns a pointer to the employee or
nilif not found. Optional style improvement:
- Line 53: Parentheses around
ifconditions are not idiomatic in Go.Apply this diff for Go style conventions:
- if(m.Employees[i].ID == id){ + if m.Employees[i].ID == id { return &m.Employees[i] }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-3/submissions/AlexO-85/solution-template.go(1 hunks)
🔇 Additional comments (4)
challenge-3/submissions/AlexO-85/solution-template.go (4)
5-10: LGTM! Employee struct is well-defined.The Employee struct with ID, Name, Age, and Salary fields is appropriate and follows Go naming conventions.
12-14: LGTM! Manager struct is correctly defined.The Manager struct appropriately contains a slice of employees.
16-19: LGTM! AddEmployee implementation is correct.The method correctly uses
appendto add employees, which is the idiomatic Go approach.
60-72: LGTM! Main function demonstrates the solution effectively.The main function appropriately exercises all Manager methods and produces correct output.
Challenge 3 Solution
Submitted by: @AlexO-85
Challenge: Challenge 3
Description
This PR contains my solution for Challenge 3.
Changes
challenge-3/submissions/AlexO-85/solution-template.goTesting
Thank you for reviewing my submission! 🚀