Kivanc10
commented
3 years ago First of all, your response is so important to me. Thank you for that. Your suggestions are so worthy and logical for me because I'm new to Go so I didn't realize the defects of that app during the development time. But when I read your message, I understand what ı should do clearly in that project. So I will try to improve my apps based on that suggestions anymore. I want to thank you again for your detailed response.
22 Ağu 2021 Paz 05:37 tarihinde Matt Brittan @.***> şunu yazdı:
Followed your Reddit link and took a look; your Reddit post there did not ask for suggestions but, having scanned your dbOp package I do have a few so thought raising an issue might be the best way to communicate (feel free to ignore/delete this). I've only looked at the dbOp package:
- You are calling sql.Open with every call. Consider doing this once at startup (as the docs https://pkg.go.dev/database/sql#Open say "...the Open function should be called just once. It is rarely necessary to close a DB."). sql.DB will reuse connections (and establish new ones where needed) which avoids the cost that comes with establishing lots of connections.
- Consider adding defer rows.Close() https://pkg.go.dev/database/sql#Rows.Close when running a query (this is called automatically when rows.Next() returns false but you may not always fully read the rows e.g. GetLastLoginToken). Also consider checking for errors - rows.Err() https://pkg.go.dev/database/sql#Rows.Err
- sql.Prepare is really only adds value when you will be running the same statement multiple times (with different parameters).
- Consider returning error more often; its nice to be able to let the end user know when something has gone wrong (and calling panic when, for example, the username is not found will not work in production).
- Consider storing a password hash (e.g. bcrypt https://pkg.go.dev/golang.org/x/crypto/bcrypt) rather than the password itself (OWASP cheatssheet https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html ).
- A number of the functions take the request body (reqBody []byte) and parse it; this does not really seem to fit within the database package (a significant part of the rationale for having a separate package is that there may be other users and these might not accept JSON). A common approach is to have a model package.
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Kivanc10/golang-rest-api-with-mysql/issues/1, or unsubscribe https://github.com/notifications/unsubscribe-auth/AMK2O5PSUQV5VJTIQPX6BQLT6BPE3ANCNFSM5CSOW5FA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email .
MattBrittan
Followed your Reddit link and took a look; your Reddit post there did not ask for suggestions but, having scanned your
dbOppackage I do have a few so thought raising an issue might be the best way to communicate (feel free to ignore/delete this). I've only looked at thedbOppackage:sql.Openwith every call. Consider doing this once at startup (as the docs say "...the Open function should be called just once. It is rarely necessary to close a DB.").sql.DBwill reuse connections (and establish new ones where needed) which avoids the cost that comes with establishing lots of connections.defer rows.Close()when running a query (this is called automatically whenrows.Next()returns false but you may not always fully read the rows e.g.GetLastLoginToken). Also consider checking for errors -rows.Err()sql.Prepareis really only adds value when you will be running the same statement multiple times (with different parameters).errormore often; its nice to be able to let the end user know when something has gone wrong (and callingpanicwhen, for example, the username is not found will not work in production).reqBody []byte) and parse it; this does not really seem to fit within the database package (a significant part of the rationale for having a separate package is that there may be other users and these might not accept JSON). A common approach is to have amodelpackage.