From ec4588063cba64a3e8fd2b78787a612a248b0b8b Mon Sep 17 00:00:00 2001 From: Bhanu Date: Wed, 17 Jan 2024 21:07:39 +0530 Subject: [PATCH 1/2] Added review comments --- api.go | 9 +++++++++ main.go | 21 +++++++++++++++++++++ utils.go | 27 +++++++++++++++++++++++++++ 3 files changed, 57 insertions(+) diff --git a/api.go b/api.go index cbef2ef..f46a788 100644 --- a/api.go +++ b/api.go @@ -5,15 +5,24 @@ import ( "net/http" ) +// Don't add all functionalities into a single function. +// Follow the single Responsibility rule. +// This also helps in writing the test functions easily. +// Move the code to a seperate package called handlers instead of defining them in main package func setupJsonApi() { + // The Handlers can be moved to a seperate package http.HandleFunc("/createUser", func(w http.ResponseWriter, r *http.Request) { // create mysql connection + // Instead of creating a connection everytime, Create it once and store it in a structure so the same connection can be accessed whenever needed conn := createConnection() name := r.FormValue("name") email := r.FormValue("email") + // Passing the Query in this format can cause SQL Injections, You can avoid this by using parameterized queries. query := "INSERT INTO users (name, email) VALUES (" + name + ", " + email + ")" result, err := conn.Exec(query) + // Again, Error shouldn't be handled like this as mentioned in utils.go fmt.Println("result ", result, " err ", err.Error()) + // Use Proper http status codes and set the http headers properly w.Write([]byte("Created user successfully!")) }) http.HandleFunc("/updateUser", func(w http.ResponseWriter, r *http.Request) { diff --git a/main.go b/main.go index 18805dd..aa54082 100644 --- a/main.go +++ b/main.go @@ -4,7 +4,28 @@ import ( "net/http" ) +// Proper Comments should be added to the code wherever needed, It Improves the readability and the maintainability of the code. func main() { + + // Follow the go naming convention + // Use small letters when you don't want to access a function or variable outside the package and Captital letter when you want to access it outside the package + // All the Acronyms used should be in Capital letters. + // Name of the shouldn't be long enough and should tell you precisely what it does. setupJsonApi() + + // Use Middlewares to add additional functionalities. + // Add CSRF Protection using a middleware + // Use routing packages to handle the routes + + // Use a logger package to debug the code easily, + // You can use the standard log library from go or a third party logging package like zap, logrun or Zerolog Which give you additional features like setting the log level. + + /* + Do not hard code values. + Hard coding values can make the code less flexible and requires changes in code if the value needs to be modified in future. + Instead of Hard coding values use flag package to pass the value as a command line argument. + You can also use environment variables to set the values + */ http.ListenAndServe(":80", nil) + } diff --git a/utils.go b/utils.go index e65cd87..f460556 100644 --- a/utils.go +++ b/utils.go @@ -7,7 +7,34 @@ import ( // createConnection creates a connection to mysql database func createConnection() *sql.DB { + // Don't Hard code values as like mentioned earlier. + // The password shouldn't hardcoded in the connection string as this is not a good practice. + // Create a structure in go and store all the values required to make the db connection. + /* + type Database struct { + username string, + password string, + port uint32, + addr string, + dbname string, + } + + you can pass this structure as a method receiver to all the methods + ex. func(db *Database) CreateConnection() *sql.DB {} + + */ db, err := sql.Open("mysql", "root:password@tcp(127.0.0.1:3306)/test") + // Don't print the errors directly + /* Errors in golang are handled like this + if err != nil { + // Print the error or return the error + } + */ fmt.Println("sql open " + err.Error()) return db } + +// Use a separate package for database related methods. +// Don't combile all the functinalties into a single package +// Add Additional functions to add, update and delete data from a table +// Close the db connection after using it. From 6ce1b59d4db4e2f059cb92b938f5f695f6058d56 Mon Sep 17 00:00:00 2001 From: bhanu-mL Date: Thu, 18 Jan 2024 20:41:10 +0530 Subject: [PATCH 2/2] Modified comments and added few examples --- main.go | 8 ++++++-- utils.go | 14 ++++++++++---- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/main.go b/main.go index aa54082..0ddfbf1 100644 --- a/main.go +++ b/main.go @@ -18,13 +18,17 @@ func main() { // Use routing packages to handle the routes // Use a logger package to debug the code easily, - // You can use the standard log library from go or a third party logging package like zap, logrun or Zerolog Which give you additional features like setting the log level. + // You can use the standard log library from go or a third party logging package like zap, logrun or Zerolog Which give you additional features like setting the log level, etc. /* Do not hard code values. Hard coding values can make the code less flexible and requires changes in code if the value needs to be modified in future. + Instead of Hard coding values use flag package to pass the value as a command line argument. - You can also use environment variables to set the values + Ex. var port = flag.Int("p",8080, "Enter the Port Number on which you want to run the application") + + You can also use environment variables to get the values + Ex. os.Getenv("APP_PORT") to get the port number */ http.ListenAndServe(":80", nil) diff --git a/utils.go b/utils.go index f460556..19b8907 100644 --- a/utils.go +++ b/utils.go @@ -7,8 +7,8 @@ import ( // createConnection creates a connection to mysql database func createConnection() *sql.DB { - // Don't Hard code values as like mentioned earlier. - // The password shouldn't hardcoded in the connection string as this is not a good practice. + // Don't Hard code values as mentioned earlier. + // The password shouldn't be hardcoded in the connection string as this is not secure. // Create a structure in go and store all the values required to make the db connection. /* type Database struct { @@ -17,17 +17,23 @@ func createConnection() *sql.DB { port uint32, addr string, dbname string, + SQLcon *sql.DB } + // Additionally Create a Init function to initialize the structure + ex. + func InitDB(usr, pswd, addr string, port uint32) *Database{} {} + you can pass this structure as a method receiver to all the methods - ex. func(db *Database) CreateConnection() *sql.DB {} + ex. + func(db *Database) CreateConnection() error {} */ db, err := sql.Open("mysql", "root:password@tcp(127.0.0.1:3306)/test") // Don't print the errors directly /* Errors in golang are handled like this if err != nil { - // Print the error or return the error + return fmt.Errorf("DB Connection Error:", err) } */ fmt.Println("sql open " + err.Error())