-
-
Notifications
You must be signed in to change notification settings - Fork 752
Add solution for Challenge 3 by cckwes #848
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 introduces an HR management model with Employee and Manager types. Manager implements methods to add/remove employees, compute average salary, find employees by ID, and includes a main function demonstrating basic usage. 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/cckwes/solution-template.go (3)
21-29: Slice‑based removal is correct; consider signaling success/failureThe index lookup +
append(m.Employees[:foundIndex], m.Employees[foundIndex+1:]...)pattern is correct and safe, including for first/last elements. Depending on the challenge requirements, you might optionally return aboolto indicate whether an employee was actually removed, which can simplify caller logic.
46-64: FindEmployeeByID + helper are fine; be aware of pointer semanticsThe helper
findEmployeeIndexByIDis clear, andFindEmployeeByIDreturningnilon miss is straightforward. Returning*Employeemeans callers can mutate the stored employee via the pointer, and the pointer may become stale if employees are later removed/reordered—this is acceptable but worth being conscious of; alternatively, you could return anEmployeevalue with a secondbool/errorresult if you want stronger encapsulation.
1-78: Run gofmt for canonical Go formattingThere’s a minor mix of tabs/spaces in a few function bodies; running
gofmton this file will normalize indentation and keep it consistent with standard Go formatting, which is generally expected for Go submissions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
challenge-3/submissions/cckwes/solution-template.go(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
challenge-3/submissions/cckwes/solution-template.go (1)
challenge-13/submissions/Gandook/solution-template.go (1)
ID(144-144)
🔇 Additional comments (4)
challenge-3/submissions/cckwes/solution-template.go (4)
5-14: Employee / Manager structs are straightforward and idiomaticThe data modeling for
EmployeeandManageris clear and idiomatic for this scale; using a slice forEmployeeskeeps things simple and readable.
16-19: AddEmployee implementation looks goodPointer receiver plus
appendis the right choice here; this method is minimal and correct.
31-44: Average salary calculation handles empty slice correctlyGuarding against
len(m.Employees) == 0avoids divide‑by‑zero, and the loop is clear. Returning0for “no employees” is a reasonable convention; just ensure it matches the challenge spec.
66-78: main is a nice demo; double‑check it matches the challenge harness expectationsThe example usage in
maincorrectly exercises all the methods and prints sensible output. For this repo’s challenges, sometimes the tests expect only library code (nomain) or a specific package/interface shape, so please confirm the challenge instructions to ensure havingpackage mainandfunc main()in this file won’t interfere with automated tests.
Challenge 3 Solution
Submitted by: @cckwes
Challenge: Challenge 3
Description
This PR contains my solution for Challenge 3.
Changes
challenge-3/submissions/cckwes/solution-template.goTesting
Thank you for reviewing my submission! 🚀