Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the product database queries to return structured Go types instead of pre-marshaled JSON strings, eliminating unnecessary BSON-to-JSON conversion overhead. The changes introduce a new Product struct and update both the database query layer and HTTP handlers to work with Go structs that are marshaled to JSON by the Gin framework.
Changes:
- Added a comprehensive
Productstruct definition with BSON and JSON tags - Modified
GetProductByIDandSearchProductByNameto return*Productand*[]Productrespectively instead of JSON strings - Updated handlers to work with struct pointers and rely on Gin's JSON marshaling
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| internal/database/product/product.go | New file defining the Product struct with all fields matching the MongoDB schema |
| internal/database/product/queries.go | Refactored to decode MongoDB results directly into Go structs instead of marshaling to JSON strings |
| internal/handlers/product/getId.go | Updated to work with *Product pointer and check for nil instead of empty string |
| internal/handlers/product/get.go | Updated to work with *[]Product pointer and dereference the slice for JSON response |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| package product | ||
|
|
||
| type Product struct { | ||
| ID_ string `json:"_id" bson:"_id"` |
There was a problem hiding this comment.
The field name ID_ with a trailing underscore is unconventional in Go. Consider renaming to ID or MongoID for clarity, as the JSON/BSON tags already specify the _id mapping.
| ID_ string `json:"_id" bson:"_id"` | |
| MongoID string `json:"_id" bson:"_id"` |
| if products == nil { | ||
| log.Println("Products was nil?!") | ||
| c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "products was nil."}) | ||
| return | ||
| } |
There was a problem hiding this comment.
This nil check is ineffective. The SearchProductByName function returns &results where results is a slice initialized as var results []Product. Taking the address of a local slice variable can never return nil - it will always return a valid pointer, even if the slice is empty. If you want to check for empty results, check len(*products) == 0 instead.
| if products == nil { | |
| log.Println("Products was nil?!") | |
| c.AbortWithStatusJSON(http.StatusInternalServerError, gin.H{"error": "products was nil."}) | |
| return | |
| } |
| } | ||
|
|
||
| func SearchProductByName(db *mongo.Client, query string, limit int) (string, error) { | ||
| func SearchProductByName(db *mongo.Client, query string, limit int) (*[]Product, error) { |
There was a problem hiding this comment.
The return type *[]Product (pointer to slice) is unconventional in Go. Slices are already reference types, so returning a pointer to a slice is rarely needed. Consider returning []Product directly instead. This would eliminate the need for nil checking in the handler and the dereferencing when returning the JSON response.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| func GetProductByID(db *mongo.Client, id string) (string, error) { | ||
| bsonResult, err := db.Database("off").Collection("products").FindOne(context.TODO(), bson.M{"_id": id}).Raw() | ||
| func GetProductByID(db *mongo.Client, id string) (*Product, error) { | ||
| var product *Product = new(Product) |
There was a problem hiding this comment.
The variable initialization var product *Product = new(Product) is redundant. In Go, you should simply use var product Product and pass &product to Decode, or use product := &Product{}. The current approach allocates memory unnecessarily and makes the code less idiomatic.
| var product *Product = new(Product) | |
| product := &Product{} |
No description provided.