Add solution for gin challenge-1-basic-routing by RuslanG003#1469
Conversation
WalkthroughThis PR introduces a complete Gin-based HTTP API implementation for a basic routing challenge. It defines User and Response types, establishes in-memory user storage, and implements CRUD operations with a search endpoint, complete with validation and error handling. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 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. Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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: 5
🧹 Nitpick comments (2)
packages/gin/challenge-1-basic-routing/submissions/RuslanG003/solution.go (2)
30-36: Race condition on global state.The
usersslice andnextIDcounter are modified by multiple handlers (createUser,updateUser,deleteUser) without synchronization. Concurrent HTTP requests will cause data races.For a challenge solution this may be acceptable, but in production code you'd need a
sync.Mutexorsync.RWMutexto protect access.🔒 Proposed fix using mutex
+import "sync" + // In-memory storage +var usersMu sync.RWMutex 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 = 4Then use
usersMu.RLock()/usersMu.RUnlock()for reads andusersMu.Lock()/usersMu.Unlock()for writes in handlers.
51-51: Handle error fromrouter.Run().The error returned by
Run()is ignored. If the server fails to start (e.g., port already in use), the program will silently exit.Proposed fix
- router.Run(":8080") // TODO: Start server on http://localhost:8080 + if err := router.Run(":8080"); err != nil { + panic(err) + }
| } | ||
| var nextID = 4 | ||
|
|
||
| //main.go starts herez |
There was a problem hiding this comment.
Fix typo in comment.
"herez" should be "here".
-//main.go starts herez
+//main.go starts here📝 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.
| //main.go starts herez | |
| //main.go starts here |
| c.JSON(http.StatusOK, Response{ | ||
| Success: true, | ||
| Data: updatedUser, | ||
| Message: "User updated Succsesfully", |
There was a problem hiding this comment.
Fix typo in message.
"Succsesfully" should be "Successfully".
- Message: "User updated Succsesfully",
+ Message: "User updated successfully",📝 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.
| Message: "User updated Succsesfully", | |
| Message: "User updated successfully", |
| if c == nil { | ||
| c.JSON(http.StatusBadRequest, Response{ | ||
| Success: false, | ||
| Error: "User not found", | ||
| Code: http.StatusBadRequest, | ||
| }) | ||
| return | ||
| } |
There was a problem hiding this comment.
Remove dead code: nil check on gin.Context.
This check is ineffective—if c were nil, calling c.JSON() inside the block would panic. Gin always passes a valid *gin.Context to handlers; this code is unreachable.
Proposed fix
func deleteUser(c *gin.Context) {
- // TODO: Get user ID from path
- if c == nil {
- c.JSON(http.StatusBadRequest, Response{
- Success: false,
- Error: "User not found",
- Code: http.StatusBadRequest,
- })
- return
- }
userID, err := strconv.Atoi(c.Param("id"))| users = append(users[:index], users[index+1:]...) | ||
| c.JSON(http.StatusOK, Response{ | ||
| Success: true, | ||
| Message: "User deleted Succsesfully", |
There was a problem hiding this comment.
Fix typo in message.
Same typo as in updateUser: "Succsesfully" should be "Successfully".
- Message: "User deleted Succsesfully",
+ Message: "User deleted successfully",📝 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.
| Message: "User deleted Succsesfully", | |
| Message: "User deleted successfully", |
| if matched, _ := regexp.MatchString(`^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$`, user.Email); !matched { | ||
| return errors.New("Email is invalid") | ||
| } |
There was a problem hiding this comment.
Email validation fails on uppercase characters.
The regex ^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$ only matches lowercase. Valid emails like "John@Example.COM" will fail validation.
Proposed fix: normalize email before validation
- if matched, _ := regexp.MatchString(`^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$`, user.Email); !matched {
+ if matched, _ := regexp.MatchString(`^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$`, strings.ToLower(user.Email)); !matched {
return errors.New("Email is invalid")
}Additionally, consider compiling the regex once at package level to avoid recompilation on every call:
var emailRegex = regexp.MustCompile(`^[a-z0-9._%+-]+@[a-z0-9.-]+\.[a-z]{2,}$`)
2 participants
gin challenge-1-basic-routing Solution
Submitted by: @RuslanG003
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/RuslanG003/solution.goTesting
Thank you for reviewing my submission! 🚀