Add solution for gin challenge-1-basic-routing by Miniuzzo#1461
Add solution for gin challenge-1-basic-routing by Miniuzzo#1461Miniuzzo wants to merge 1 commit intoRezaSi:mainfrom
Conversation
WalkthroughIntroduces a Go-based Gin HTTP API scaffold with an in-memory user store. Adds User and Response structs for API data. Implements handlers for fetching all users, fetching by ID, creating, updating, deleting, and searching users with basic validation and error handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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
🧹 Nitpick comments (2)
packages/gin/challenge-1-basic-routing/submissions/Miniuzzo/solution.go (2)
225-237: Finish the helper implementations and reuse them to remove duplication
findUserByIDandvalidateUserare TODO placeholders. Implementing and using them ingetUserByID/createUser/updateUserwill reduce repeated lookup/validation logic and avoid drift.If you want, I can draft a compact refactor patch that centralizes lookup + validation and updates handlers accordingly.
Based on learnings: Do not modify predefined function signatures in submission solutions; keep challenge-required signatures intact.
51-51: Handlerouter.Runerror explicitlyLine 51 ignores startup failure (e.g., port bind error), which can mask broken boot.
Suggested fix
- router.Run() + if err := router.Run(":8080"); err != nil { + panic(err) + }
| var users = []User{ | ||
| {ID: 1, Name: "John Doe", Email: "john@example.com", Age: 30}, | ||
| {ID: 2, Name: "Jane Smith", Email: "jane@example.com", Age: 25}, | ||
| {ID: 3, Name: "Bob Wilson", Email: "bob@example.com", Age: 35}, | ||
| } | ||
| var nextID = 4 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find packages/gin/challenge-1-basic-routing/submissions/Miniuzzo -name "solution.go" -type fRepository: RezaSi/go-interview-practice
Length of output: 143
🏁 Script executed:
cat -n packages/gin/challenge-1-basic-routing/submissions/Miniuzzo/solution.goRepository: RezaSi/go-interview-practice
Length of output: 7110
Protect shared in-memory state from concurrent request races
users and nextID are global mutable state accessed by multiple handlers. Under concurrent requests, this causes data races and possible slice corruption/lost updates.
Suggested fix (mutex around reads/writes)
import (
+ "sync"
"strconv"
"strings"
"github.com/gin-gonic/gin"
)
@@
var users = []User{
@@
}
var nextID = 4
+var usersMu sync.RWMutex func getAllUsers(c *gin.Context) {
+ usersMu.RLock()
+ defer usersMu.RUnlock()
res := Response{
Success: true,
Data: users,
}
@@
func createUser(c *gin.Context) {
@@
+ usersMu.Lock()
+ defer usersMu.Unlock()
newUser.ID = nextID
nextID++
users = append(users, newUser)
@@
func updateUser(c *gin.Context) {
@@
+ usersMu.Lock()
+ defer usersMu.Unlock()
for i := range users {
@@
func deleteUser(c *gin.Context) {
@@
+ usersMu.Lock()
+ defer usersMu.Unlock()
for i := range users {Also applies to: 114-115, 118-118, 152-155, 182-182
| // TODO: Setup routes | ||
| // GET /users - Get all users | ||
| router.GET("/users", getAllUsers) | ||
| // GET /users/:id - Get user by ID | ||
| router.GET("/users/:id", getUserByID) | ||
| // POST /users - Create new user | ||
| router.POST("/users", createUser) | ||
| // PUT /users/:id - Update user | ||
| // DELETE /users/:id - Delete user | ||
| // GET /users/search - Search users by name | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n packages/gin/challenge-1-basic-routing/submissions/Miniuzzo/solution.goRepository: RezaSi/go-interview-practice
Length of output: 7110
Wire the declared handlers in main
The handlers updateUser, deleteUser, and searchUsers are fully implemented (lines 129, 168, 198) but not registered to routes. Lines 46–48 contain only comments, leaving these endpoints unreachable.
Suggested fix
// PUT /users/:id - Update user
+ router.PUT("/users/:id", updateUser)
// DELETE /users/:id - Delete user
+ router.DELETE("/users/:id", deleteUser)
// GET /users/search - Search users by name
+ router.GET("/users/search", searchUsers)📝 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.
| // TODO: Setup routes | |
| // GET /users - Get all users | |
| router.GET("/users", getAllUsers) | |
| // GET /users/:id - Get user by ID | |
| router.GET("/users/:id", getUserByID) | |
| // POST /users - Create new user | |
| router.POST("/users", createUser) | |
| // PUT /users/:id - Update user | |
| // DELETE /users/:id - Delete user | |
| // GET /users/search - Search users by name | |
| // TODO: Setup routes | |
| // GET /users - Get all users | |
| router.GET("/users", getAllUsers) | |
| // GET /users/:id - Get user by ID | |
| router.GET("/users/:id", getUserByID) | |
| // POST /users - Create new user | |
| router.POST("/users", createUser) | |
| // PUT /users/:id - Update user | |
| router.PUT("/users/:id", updateUser) | |
| // DELETE /users/:id - Delete user | |
| router.DELETE("/users/:id", deleteUser) | |
| // GET /users/search - Search users by name | |
| router.GET("/users/search", searchUsers) |
| if err != nil { | ||
| c.JSON(404, Response{Success: false, Message: "Invalid ID format", Error: err.Error()}) | ||
| return | ||
| } | ||
|
|
||
| var user User | ||
| if err := c.ShouldBindJSON(&user); err != nil { | ||
| c.JSON(404, Response{ | ||
| Success: false, | ||
| Message: "Invalid JSON format", | ||
| Error: err.Error(), | ||
| }) |
There was a problem hiding this comment.
updateUser can overwrite identity and returns wrong status for bad input
Line 137 and Line 143 return 404 for malformed ID/JSON; these should be 400.
Also, Line 154 replaces the stored record with request body as-is, so path ID can be lost or changed.
Suggested fix
id, err := strconv.Atoi(idStr)
if err != nil {
- c.JSON(404, Response{Success: false, Message: "Invalid ID format", Error: err.Error()})
+ c.JSON(400, Response{Success: false, Message: "Invalid ID format", Error: err.Error()})
return
}
@@
if err := c.ShouldBindJSON(&user); err != nil {
- c.JSON(404, Response{
+ c.JSON(400, Response{
Success: false,
Message: "Invalid JSON format",
Error: err.Error(),
})
return
}
@@
for i := range users {
if users[i].ID == id{
+ user.ID = id
users[i] = userAlso applies to: 152-157
| if err != nil { | ||
| c.JSON(404, Response{ | ||
| Success: false, | ||
| Message: "Invalid ID format", | ||
| Error: err.Error()}) | ||
| return |
There was a problem hiding this comment.
Use 400 (not 404) for invalid ID format in deleteUser
Line 173 currently returns 404 on parse failure. That is a bad-request input error, not not-found.
1 participant
gin challenge-1-basic-routing Solution
Submitted by: @Miniuzzo
Package: gin
Challenge: challenge-1-basic-routing
Description
This PR contains my solution for gin challenge-1-basic-routing.
Changes
packages/gin/challenge-1-basic-routing/submissions/Miniuzzo/solution.goTesting
Thank you for reviewing my submission! 🚀