Add solution for Challenge 13 by Johrespi #721
Conversation
WalkthroughIntroduces a ProductStore system in Go with SQLite backing, featuring a Product struct and ProductStore that encapsulates database operations. Implements database initialization, CRUD operations (CreateProduct, GetProduct, UpdateProduct, DeleteProduct), product listing with optional category filtering, and transactional batch inventory updates using prepared statements. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| for rows.Next() { | ||
| var product Product | ||
| err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| products = append(products, &product) | ||
| } | ||
|
|
||
| return products, nil |
There was a problem hiding this comment.
Surface iteration errors from rows.Err()
If the driver reports an error after rows.Next() stops, it’s currently ignored and you return partial data. Add a rows.Err() check before returning to propagate those failures.
Apply this diff:
for rows.Next() {
product := &Product{}
if err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category); err != nil {
return nil, err
}
products = append(products, product)
}
+
+ if err := rows.Err(); err != nil {
+ return nil, err
+ }
return products, nil📝 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.
| for rows.Next() { | |
| var product Product | |
| err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category) | |
| if err != nil { | |
| return nil, err | |
| } | |
| products = append(products, &product) | |
| } | |
| return products, nil | |
| for rows.Next() { | |
| product := &Product{} | |
| if err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category); err != nil { | |
| return nil, err | |
| } | |
| products = append(products, product) | |
| } | |
| if err := rows.Err(); err != nil { | |
| return nil, err | |
| } | |
| return products, nil |
🤖 Prompt for AI Agents
In challenge-13/submissions/Johrespi/solution-template.go around lines 166 to
175, the code iterates rows and returns products without checking for any error
reported by the driver after iteration; add a check for rows.Err() after the for
loop and before returning to surface any iteration errors and return that error
(or wrap it) instead of silently returning partial results.
| for rows.Next() { | ||
| var product Product | ||
| err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category) | ||
| if err != nil { | ||
| return nil, err | ||
| } | ||
| products = append(products, &product) | ||
| } |
There was a problem hiding this comment.
Fix pointer reuse in ListProducts
product is a loop-scoped value, so every appended pointer ends up referencing the same memory and the slice returns multiple aliases of the last row. Please allocate a new struct per iteration before scanning to guarantee each entry points to its own product instance.
Apply this diff:
- for rows.Next() {
- var product Product
- err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category)
- if err != nil {
- return nil, err
- }
- products = append(products, &product)
- }
+ for rows.Next() {
+ product := &Product{}
+ if err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category); err != nil {
+ return nil, err
+ }
+ products = append(products, product)
+ }📝 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.
| for rows.Next() { | |
| var product Product | |
| err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category) | |
| if err != nil { | |
| return nil, err | |
| } | |
| products = append(products, &product) | |
| } | |
| for rows.Next() { | |
| product := &Product{} | |
| if err := rows.Scan(&product.ID, &product.Name, &product.Price, &product.Quantity, &product.Category); err != nil { | |
| return nil, err | |
| } | |
| products = append(products, product) | |
| } |
🤖 Prompt for AI Agents
challenge-13/submissions/Johrespi/solution-template.go around lines 166 to 173:
the loop reuses a single loop-scoped variable "product" and appends its pointer
each iteration, causing all slice entries to point to the same struct; fix by
allocating a new Product instance inside the loop for each row (e.g., create a
new variable or take address of a fresh struct) and scan into that fresh
instance before appending so each slice element references its own product.
| tx, err := ps.db.Begin() | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| defer func() { | ||
| if err != nil { | ||
| tx.Rollback() | ||
| } | ||
| }() | ||
|
|
||
| stmt := `UPDATE products SET quantity = ? WHERE id = ?` | ||
|
|
||
| for productID, newQuantity := range updates { | ||
| result, err := tx.Exec(stmt, newQuantity, productID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| rowsAffected, err := result.RowsAffected() | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if rowsAffected == 0 { | ||
| return fmt.Errorf("product with ID %d not found", productID) | ||
| } | ||
|
|
||
| } | ||
| return tx.Commit() |
There was a problem hiding this comment.
Guarantee rollback when Commit fails
Because the function returns tx.Commit() directly, err stays nil and the deferred rollback never runs on commit failure. Capture the commit error first, run an explicit rollback on failure, and only then return. Converting to a named return makes the defer pattern work as intended.
Apply this diff:
-func (ps *ProductStore) BatchUpdateInventory(updates map[int64]int) error {
+func (ps *ProductStore) BatchUpdateInventory(updates map[int64]int) (err error) {
tx, err := ps.db.Begin()
if err != nil {
return err
}
defer func() {
if err != nil {
- tx.Rollback()
+ _ = tx.Rollback()
}
}()
stmt := `UPDATE products SET quantity = ? WHERE id = ?`
for productID, newQuantity := range updates {
- result, err := tx.Exec(stmt, newQuantity, productID)
- if err != nil {
- return err
- }
+ result, execErr := tx.Exec(stmt, newQuantity, productID)
+ if execErr != nil {
+ err = execErr
+ return err
+ }
- rowsAffected, err := result.RowsAffected()
- if err != nil {
- return err
- }
+ rowsAffected, rowsErr := result.RowsAffected()
+ if rowsErr != nil {
+ err = rowsErr
+ return err
+ }
if rowsAffected == 0 {
- return fmt.Errorf("product with ID %d not found", productID)
+ err = fmt.Errorf("product with ID %d not found", productID)
+ return err
}
}
- return tx.Commit()
+ if commitErr := tx.Commit(); commitErr != nil {
+ _ = tx.Rollback()
+ return commitErr
+ }
+ return nil
}Committable suggestion skipped: line range outside the PR's diff.
2 participants
Challenge 13 Solution
Submitted by: @Johrespi
Challenge: Challenge 13
Description
This PR contains my solution for Challenge 13.
Changes
challenge-13/submissions/Johrespi/solution-template.goTesting
Thank you for reviewing my submission! 🚀