Implementing a REST API in Go using gorilla/mux and Gorm While learning Go I decided to implement a simple REST API to get to know the language and since I am inexperienced in Go I would appreciate any feedback on the code.

Please post any thoughts you have. They don't have to be constructive, but if you have some constructive feedback that will be great.

Q: In particular, I would like to ask the next questions: Are there any bad patterns? Is there something you would do another way? What could be improved? Do I violate any code style rules (say, type names, variables)? Is it ok to place all this code in one file?

Note: I used gorilla/mux and gorm here.

type student struct {
    ID   string `gorm:"primary_key" json:"id"`
    Name string `json:"name"`
    Age  int    `json:"age"`
}

type App struct {
    DB *gorm.DB
}

func (a *App) start() {
    db, err := gorm.Open(
        "postgres",
        "user=go password=go dbname=go sslmode=disable")
    if err != nil {
        panic(err.Error())
    }
    a.DB = db
    db.AutoMigrate(&amp;student{})
    r := mux.NewRouter()
    r.HandleFunc("/students", a.getAllStudents).Methods("GET")
    r.HandleFunc("/students", a.addStudent).Methods("POST")
    r.HandleFunc("/students/{id}", a.updateStudent).Methods("PUT")
    r.HandleFunc("/students/{id}", a.deleteStudent).Methods("DELETE")
    http.ListenAndServe(":8080", r)
}

func (a *App) getAllStudents(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    var all []student
    err := a.DB.Find(&amp;all).Error
    if err != nil {
        sendErr(w, http.StatusInternalServerError, err.Error())
    } else {
        json.NewEncoder(w).Encode(all)
    }
}

func (a *App) addStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    err := json.NewDecoder(r.Body).Decode(&amp;s)
    if err != nil {
        sendErr(w, http.StatusBadRequest, err.Error())
    } else {
        err = a.DB.Save(&amp;s).Error
        if err != nil {
            sendErr(w, http.StatusInternalServerError, err.Error())
        } else {
            w.WriteHeader(http.StatusCreated)
        }
    }
}

func (a *App) updateStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    err := json.NewDecoder(r.Body).Decode(&amp;s)
    if err != nil {
        sendErr(w, http.StatusBadRequest, err.Error())
    } else {
        s.ID = mux.Vars(r)["id"]
        err = a.DB.Save(&amp;s).Error
        if err != nil {
            sendErr(w, http.StatusInternalServerError, err.Error())
        }
    }
}

func (a *App) deleteStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    a.DB.First(&amp;s, mux.Vars(r)["id"])
    err := a.DB.Delete(s).Error
    if err != nil {
        sendErr(w, http.StatusInternalServerError, err.Error())
    }
}

func sendErr(w http.ResponseWriter, code int, message string) {
    response, _ := json.Marshal(map[string]string{"error": message})
    w.Header().Set("Content-Type", "application/json")
    w.WriteHeader(code)
    w.Write(response)
}

func main() {
    app := App{}
    app.start()
}


Update: I've got some great feedback and I have an additional question related to that. In both the answers you suggest me to get rid of else blocks / use an early return. Is it a common practice in Go? Is it because of error handling in Go which makes you create more if/else blocks? I am asking because in other languages (such as Java) it is considered a bad practice. The main reason is that it hides complexity. Every time you create a return inside an if block you make a the code more flat hiding the real complexity.
ID as a string is weird. I expect to see an int here to benefit of the auto increment feature.
your delete handler is weak. Suppose the GET ID parameter is set to a non existing ID value, the read operation will fail but the value of the ID property of the student variable will be an empty string. The next delete statement will delete the row with ID=="", which might exists and be a valid. The problem also exists for an integer PK where ID=0. You must check all errors.
the update handler has a similar problem, and it is not acceptable for as long as the api provides both add and update as different operations. Usually an update operation requires to check against a couple PK+LastUpdateDate to ensure acid transactions from UI POV. So, you might try to load the entity from the db, at the cost of an extra read. Or update with clauses. The latter might be done automatically by the db framework, this is unclear without a better knowledge of its internal.
the getAllStudents handler is a ddos. Obviously that always depends of the db size, but for a production system there must not exist any handler that loads the entire db in memory. So add Limit/Offset parameters, check they are valid and apply them to the query.
overall your code has too many level of indentations. You should practice the early return pattern to improve its clarity.


Here is an updated version of the code, so the reader can evaluate the return early pattern and appreciate various modifications as explained before.


Care was taken to demonstrate a proper startup/shutdown sequence to prevent connections misshandling.
http.Server is moved out so the handler is mountable onto alternatives http server implementation (httptest.*)
the db engine is switched to sqlite for demonstration
OP was missing the pgsql driver blank import


here is the code (it is still missing proper test suite!)

package main

import (
    "context"
    "encoding/json"
    "errors"
    "fmt"
    "log"
    "net/http"
    "os"
    "os/signal"
    "strconv"
    "time"

    "github.com/gorilla/mux"
    "github.com/jinzhu/gorm"
    _ "github.com/jinzhu/gorm/dialects/sqlite"
)

type student struct {
    ID   string `gorm:"primary_key" json:"id"`
    Name string `json:"name"`
    Age  int    `json:"age"`

    // http://gorm.io/docs/conventions.html
    // gorm.Model is a basic GoLang struct which includes the following fields: ID, CreatedAt, UpdatedAt, DeletedAt.
    CreatedAt time.Time
    UpdatedAt time.Time
    DeletedAt *time.Time
}

type App struct {
    DB *gorm.DB
}

func (a *App) Handler() http.Handler {
    r := mux.NewRouter()
    r.HandleFunc("/students", a.getAllStudents).Methods("GET")
    r.HandleFunc("/students", a.addStudent).Methods("POST")
    r.HandleFunc("/students/{id}", a.updateStudent).Methods("PUT")
    r.HandleFunc("/students/{id}", a.deleteStudent).Methods("DELETE")
    return r
}

func (a *App) Open() error {
    db, err := gorm.Open("sqlite3", ":memory:")
    if err != nil {
        return err
    }
    a.DB = db
    return db.AutoMigrate(&amp;student{}).Error
}

func (a *App) Close() error {
    return a.DB.Close()
}

func (a *App) getAllStudents(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    // see also a struct decoder to transform get params as struct.
    // https://github.com/gorilla/schema
    var offset, limit int
    {
        y := r.URL.Query().Get("Limit")
        if y != "" {
            x, err := strconv.Atoi(y)
            if err != nil {
                sendErr(w, fmt.Errorf("Limit argument, %v", err), http.StatusBadRequest)
                return
            }
            limit = x
        }
        y = r.URL.Query().Get("Offset")
        if y != "" {
            x, err := strconv.Atoi(y)
            if err != nil {
                sendErr(w, fmt.Errorf("Offset argument, %v", err), http.StatusBadRequest)
                return
            }
            offset = x
        }
    }
    if limit &lt; 0 || limit &gt; 30 {
        limit = 30
    }
    var all []student
    err := a.DB.Find(&amp;all).Limit(limit).Offset(offset).Error
    if err != nil {
        sendErr(w, err, http.StatusInternalServerError)
        return
    }
    json.NewEncoder(w).Encode(all)
}

func (a *App) addStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    err := json.NewDecoder(r.Body).Decode(&amp;s)
    if err != nil {
        sendErr(w, err, http.StatusBadRequest)
        return
    }
    err = a.DB.Create(&amp;s).Error
    if err != nil {
        sendErr(w, err, http.StatusInternalServerError)
        return
    }
    w.WriteHeader(http.StatusCreated)
}

func (a *App) updateStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    s := student{}
    err := json.NewDecoder(r.Body).Decode(&amp;s)
    if err != nil {
        sendErr(w, err, http.StatusBadRequest)
        return
    }
    id := mux.Vars(r)["id"]
    if s.ID != id {
        sendErr(w, errors.New("invalid ID"), http.StatusBadRequest)
        return
    }
    s.ID = id                 // Why not read it from Body directly ?
    err = a.DB.Save(&amp;s).Error //gorm does a proper job http://gorm.io/docs/update.html
    sendErr(w, err, http.StatusInternalServerError)
}

func (a *App) deleteStudent(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    id := mux.Vars(r)["id"]
    s := student{}
    err := a.DB.First(&amp;s, id).Error
    if err != nil {
        sendErr(w, err)
        return
    }
    err = a.DB.Delete(s).Error
    sendErr(w, err)
}

func sendErr(w http.ResponseWriter, err error, codes ...int) {
    if err == nil {
        return
    }
    if len(codes) &lt; 1 {
        codes = append(codes, http.StatusInternalServerError)
    }
    response, _ := json.Marshal(map[string]string{"error": err.Error()})
    w.Header().Set("Content-Type", "application/json")
    http.Error(w, string(response), codes[0])
}

func main() {
    app := App{}
    if err := app.Open(); err != nil {
        log.Fatal("open failed", err)
    }
    defer app.Close()

    srv := &amp;http.Server{
        Addr:    ":8080",
        Handler: app.Handler(),
    }

    err := make(chan error)
    go func() {
        err &lt;- srv.ListenAndServe()
    }()

    select {
    case e := &lt;-err:
        log.Fatal("listen failed", e)
    case &lt;-time.After(time.Millisecond * 200):
        log.Println("app started at ", srv.Addr)
    }

    sigint := make(chan os.Signal, 1)
    signal.Notify(sigint, os.Interrupt)
    &lt;-sigint
    log.Println("got signal")

    if err := srv.Shutdown(context.Background()); err != nil {
        // Error from closing listeners, or context timeout:
        log.Printf("HTTP server Shutdown: %v", err)
    }
    log.Println("properly shutdown")
}