-
-
Notifications
You must be signed in to change notification settings - Fork 709
Add solution for Challenge 3 by Johrespi #652
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 file implementing an employee management system with Employee and Manager types. The Manager provides methods to add/remove employees, calculate average salary, and retrieve employees by ID, with a main function demonstrating usage. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 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 (4)
challenge-3/submissions/Johrespi/solution-template.go (4)
16-20: Remove TODO comment; consider duplicate ID validation.The implementation is correct, but the TODO comment on line 18 should be removed since the method is fully implemented. Optionally, consider validating for duplicate employee IDs if uniqueness is required by the challenge specification.
22-43: Remove TODO comment; implementation looks correct.The method is properly implemented with correct slice manipulation. Remove the TODO comment on line 24. The negative ID check (lines 25-27) is defensive but note that it allows ID 0 while rejecting negatives—ensure this aligns with your ID scheme.
45-59: Remove TODO comment; implementation is correct.The average salary calculation properly handles the empty list case and computes the result correctly. Remove the TODO comment on line 47.
61-71: Excellent pointer handling! Remove TODO comment.The implementation correctly returns
&m.Employees[i](line 66) instead of&e, avoiding the common pitfall of returning a pointer to the loop variable. This ensures the returned pointer remains valid. Remove the TODO comment on line 63.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-3/submissions/Johrespi/solution-template.go(1 hunks)
🔇 Additional comments (2)
challenge-3/submissions/Johrespi/solution-template.go (2)
5-14: LGTM! Clean struct definitions.The Employee and Manager structs are well-defined with appropriate field types and follow Go naming conventions.
73-85: LGTM! Good demonstration of functionality.The main function properly demonstrates all Manager methods. After removing Alice (ID 1), the average salary correctly calculates to 65000 (Bob's salary alone), and FindEmployeeByID successfully retrieves Bob's employee record.
Challenge 3 Solution
Submitted by: @Johrespi
Challenge: Challenge 3
Description
This PR contains my solution for Challenge 3.
Changes
challenge-3/submissions/Johrespi/solution-template.goTesting
Thank you for reviewing my submission! 🚀