Code review with changes

api.go

@@ -1,29 +1,87 @@
 package main
 
 import (
-	"fmt"
+	"database/sql"
+	"log"
 	"net/http"
 )
 
-func setupJsonApi() {
-	http.HandleFunc("/createUser", func(w http.ResponseWriter, r *http.Request) {
-		// create mysql connection
-		conn := createConnection()
-		name := r.FormValue("name")
-		email := r.FormValue("email")
-		query := "INSERT INTO users (name, email) VALUES (" + name + ", " + email + ")"
-		result, err := conn.Exec(query)
-		fmt.Println("result ", result, " err ", err.Error())
-		w.Write([]byte("Created user successfully!"))
-	})
-	http.HandleFunc("/updateUser", func(w http.ResponseWriter, r *http.Request) {
-		// create mysql connection
-		conn := createConnection()
-		name := r.FormValue("name")
-		email := r.FormValue("email")
-		query := "Update users set name=" + name + ", email=" + email + " where id=" + r.FormValue("id")
-		result, err := conn.Exec(query)
-		fmt.Println("result ", result, " err ", err.Error())
-		w.Write([]byte("User updated successfully!"))
-	})
+// Using database interface to abstract database operations to enable testing using mocks.
+type Database interface {
+	Exec(query string, args ...interface{}) (sql.Result, error)
+}
+
+type JSONAPIHandler struct {
+	DB Database
+}
+
+func NewJSONAPIHandler(db Database) *JSONAPIHandler {
+	return &JSONAPIHandler{DB: db}
+}
+
+func (h *JSONAPIHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) {
+	switch r.URL.Path {
+	case "/createUser":
+		h.createUserHandler(w, r)
+	case "/updateUser":
+		h.updateUserHandler(w, r)
+
+	// add more endpoints here
+	default:
+		http.NotFound(w, r)
+	}
+}
+
+func (h *JSONAPIHandler) createUserHandler(w http.ResponseWriter, r *http.Request) {
+	name := r.FormValue("name")
+	email := r.FormValue("email")
+
+	if err := ValidateUserInputs(name, email, ""); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	result, err := h.DB.Exec("INSERT INTO users (name, email) VALUES (?, ?)", name, email)
+	if err != nil {
+		log.Printf("Error creating user: %v", err)
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+
+	log.Println("CreateUser result: ", result)
+
+	w.WriteHeader(http.StatusCreated)
+	w.Write([]byte("Created user successfully!"))
+}
+
+func (h *JSONAPIHandler) updateUserHandler(w http.ResponseWriter, r *http.Request) {
+	name := r.FormValue("name")
+	email := r.FormValue("email")
+	userID := r.FormValue("id")
+
+	if err := ValidateUserInputs(name, email, userID); err != nil {
+		http.Error(w, err.Error(), http.StatusBadRequest)
+		return
+	}
+
+	result, err := h.DB.Exec("UPDATE users SET name=?, email=? WHERE id=?", name, email, userID)
+	if err != nil {
+		log.Printf("Error updating user: %v", err)
+		http.Error(w, "Internal Server Error", http.StatusInternalServerError)
+		return
+	}
+
+	log.Println("UpdateUser result: ", result)
+
+	w.WriteHeader(http.StatusOK)
+	w.Write([]byte("User updated successfully!"))
+}
+
+func setupJsonApi(db Database) {
+	// create a new JSONAPIHandler instance with the given database.
+	handler := NewJSONAPIHandler(db)
+
+	// Register the handler methods for specific endpoints.
+	http.HandleFunc("/createUser", handler.createUserHandler)
+	http.HandleFunc("/updateUser", handler.updateUserHandler)
 }

main.go

@@ -1,10 +1,25 @@
 package main
 
 import (
+	"log"
 	"net/http"
 )
 
+// Using constants is recommended over direct values
+const portAddress = ":80"
+
 func main() {
-	setupJsonApi()
-	http.ListenAndServe(":80", nil)
+	db, err := createDBConnection()
+	if err != nil {
+		log.Fatalf("Failed to connect to the database: %v", err)
+	}
+
+	// Create instance of JSONAPIHandler to handle incoming HTTP requests
+	apiHandler := NewJSONAPIHandler(db)
+	log.Printf("Server listening on %s...", portAddress)
+
+	if err := http.ListenAndServe(portAddress, apiHandler); err != nil {
+		log.Fatalf("Error starting server: %v", err)
+	}
+
 }

utils.go

@@ -2,12 +2,54 @@ package main
 
 import (
 	"database/sql"
+	"errors"
 	"fmt"
+	"log"
 )
 
-// createConnection creates a connection to mysql database
-func createConnection() *sql.DB {
-	db, err := sql.Open("mysql", "root:password@tcp(127.0.0.1:3306)/test")
-	fmt.Println("sql open " + err.Error())
-	return db
+// these should ideally be environment variables and encrypted
+const (
+	dbDriver   = "mysql"
+	dbUser     = "root"
+	dbPassword = "password"
+	dbHost     = "127.0.0.1"
+	dbPort     = "3306"
+	dbName     = "test"
+)
+
+func createDBConnection() (*sql.DB, error) {
+
+	connectionString := fmt.Sprintf("%s:%s@tcp(%s:%s)/%s", dbUser, dbPassword, dbHost, dbPort, dbName)
+
+	db, err := sql.Open(dbDriver, connectionString)
+	if err != nil {
+		log.Fatalf("Error opening database connection: %v ", err)
+		return nil, err
+	}
+	log.Printf("SQL connection opened successfully")
+
+	defer func() {
+		if err := db.Close(); err != nil {
+			log.Printf("Error closing database connection: %v ", err)
+		} else {
+			log.Printf("SQL connection closed successfully ")
+		}
+	}()
+
+	return db, nil
+}
+
+func ValidateUserInputs(name, email, userID string) error {
+	if name == "" {
+		return errors.New("Name is required")
+	}
+	if email == "" {
+		return errors.New("Email is required")
+	}
+	if userID == "" {
+		return errors.New("User ID is required")
+	}
+	// We can add further validations as needed
+
+	return nil
 }