Golang REST API that CRUDS in-memory object

Question

Hey everyone I have a very simple go REST API that is running CRUD operations on an in-memory object. I would love some feedback on how I can refactor this code, and how/if I should break this up into more than one file (main.go). I am extremely new to Go and want to learn the best way to write something like this.

Some areas that I think could use improvement:

1. The way my code is handling 500s
2. JSON Request field validation feels like I am repeating it a lot
3. File structure/organization

Any help would be greatly appreciated. Below is my code:

type Trade struct {
    ClientTradeId string `json:"client_trade_id" validate:"nonzero, min=1, max=256"`
    Date          int    `json:"date" validate:"nonzero, min=20010101, max=21000101"`
    Quantity      string `json:"quantity" validate:"regexp=^[-]?[0-9]*\\.?[0-9]+$"`
    Price         string `json:"price" validate:"nonzero, regexp=^[-]?[0-9]*\\.?[0-9]+$"`
    Ticker        string `json:"ticker" validate:"nonzero"`
}
type InternalTrade struct {
    Id string `json:"Id" validate:"nonzero"`
    Trade *Trade `json:"Trade"`
}

type TradeSubmitted struct {

    TradeId string `json:"TradeId" validate:"nonzero"`
    ClientTradeId string `json:"clientTradeId" validate:"nonzero"`

}

type Error struct {
    Message string `json:"Message"`
}


var trades []InternalTrade
var submitArray []TradeSubmitted

var (

    tradeValidator = validator.NewValidator()
)


func getTrades(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    err := json.NewEncoder(w).Encode(trades)
    if err != nil {
        e := Error{Message:"Internal Server Error"}
        w.WriteHeader(http.StatusInternalServerError)
        _ = json.NewEncoder(w).Encode(e)
        return
    }
}

func getTradeById(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    params := mux.Vars(r)
    for _, trade := range trades {
        if trade.Id == params["trade_id"] {
            _ = json.NewEncoder(w).Encode(trade)
            return
        }
    }
    e := Error{Message: "ID Not Found"}
    w.WriteHeader(http.StatusNotFound)
    if err := json.NewEncoder(w).Encode(e); err != nil {
        e := Error{Message:"Internal Server Error"}
        http.Error(w, e.Message, http.StatusInternalServerError)
    }

}

func createTrade(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")

    var tradeArray []Trade

    if err := json.NewDecoder(r.Body).Decode(&tradeArray); err != nil {
        e := Error{Message:"Not processable - Missing Required"}
        w.WriteHeader(http.StatusBadRequest)
        _ = json.NewEncoder(w).Encode(e)
        return
    }

    for _, trade := range tradeArray {
        th := trade
        if errs := tradeValidator.Validate(&th); errs != nil {
            e := Error{Message: "Bad Request - Improper Types Passed"}

            w.WriteHeader(http.StatusUnprocessableEntity)
            _ = json.NewEncoder(w).Encode(e)
            return
        }
    }

    for i := range tradeArray {
        internal := InternalTrade{
            Id:    strconv.Itoa(rand.Intn(1000000)),
            Trade: &tradeArray[i],
        }
        submit := TradeSubmitted{
            TradeId:       internal.Id,
            ClientTradeId: internal.Trade.ClientTradeId,
        }

        submitArray = append(submitArray, submit)
        trades = append(trades, internal)
    }


    if err := json.NewEncoder(w).Encode(submitArray); err != nil {
        e := Error{Message:"Internal Server Error"}
        w.WriteHeader(http.StatusInternalServerError)
        _ = json.NewEncoder(w).Encode(e)
        return
    }

}

func deleteTrade(w http.ResponseWriter, r *http.Request) {
    params := mux.Vars(r)
    for idx, trade := range trades {
        if trade.Id == params["trade_id"] {
            trades = append(trades[:idx], trades[idx+1:]...)
            w.WriteHeader(http.StatusNoContent)
            return
        }
    }
    e := Error{Message:"ID Not Found"}
    w.WriteHeader(http.StatusNotFound)
    if err := json.NewEncoder(w).Encode(e); err != nil {
        e := Error{Message:"Internal Server Error"}
        http.Error(w, e.Message, http.StatusInternalServerError)
    }

}

func updateTrade(w http.ResponseWriter, r *http.Request) {
    w.Header().Set("Content-Type", "application/json")
    params := mux.Vars(r)
    for idx, trade := range trades {
        if trade.Id == params["trade_id"] {
            trades = append(trades[:idx], trades[idx+1:]...)
            var internal InternalTrade
            _  = json.NewDecoder(r.Body).Decode(&internal.Trade)
            internal.Id = params["trade_id"]
            trades = append(trades, internal)
            json.NewEncoder(w).Encode(internal)
            return
        }
    }

    e := Error{Message:"ID Not Found"}
    w.WriteHeader(http.StatusNotFound)
    if err := json.NewEncoder(w).Encode(e); err != nil {
        e := Error{Message:"Internal Server Error"}
        http.Error(w, e.Message, http.StatusInternalServerError)
    }

}

func main() {

    router := mux.NewRouter()
    trades = append(trades, InternalTrade{ Id:"1", Trade: &Trade{ClientTradeId: "T-50264430-bc41", Date:20200101,
        Quantity:"100", Price:"10.00", Ticker:"APPL"}})
    trades = append(trades, InternalTrade{ Id:"2", Trade: &Trade{ClientTradeId: "T-99999999-ab14", Date:20200101,
        Quantity:"100", Price:"420.00", Ticker:"TSLA"}})


    router.HandleFunc("/v1/trades", getTrades).Methods("GET")
    router.HandleFunc("/v1/trades/{trade_id}", getTradeById).Methods("GET")
    router.HandleFunc("/v1/trades", createTrade).Methods("POST")
    router.HandleFunc("/v1/trades/{trade_id}", deleteTrade).Methods("DELETE")
    router.HandleFunc("/v1/trades/{trade_id}", updateTrade).Methods("PUT")
    log.Fatal(http.ListenAndServe(":8080", router))
}

Answer 1

For your first Go program this looks very good, apart from the global variables the code looks easy to read; the annotations for JSON processing and validation are fine and you're also using a standard library for the routing. Overall, doesn't raise any big issues for me.

---

Firstly though, I'd highly suggest that the trades and submitArray global variables are changed into member variables in a handler structure and secondly protected against concurrent access. The first bit will help organising code in larger projects, you almost never want globals lying around like that. The second one is simply necessary when data is modified from potentially multiple threads.

The handler functions share a lot of code for setup; it'd be good to extract some common functionality perhaps, or to use a library that deals with, say, JSON and REST routes specifically.

There's some error handling missing where the error results are explicitly discarded, that's somewhat of a bad style, though understandable for things like the JSON encoding. How about simply logging those errors at least?

Also again the JSON error handling deserves its own function to make it all less repetitive.

---

Specifically regarding your questions:

1. The error handling is fine, even though the error messages themselves could be more expressive, if you choose to not return the error message itself, at least log it for your own debugging purposes.
2. Validation looks okay to me? Apart from Data, there's a lot of dates that the validation here accepts that can never be real dates. Consider stronger validation for a real project, otherwise you might end up with lots of "month 99" in your data.
3. Looks okay, ordering this way definitely makes sense, although with more definitions you might have to split things up into multiple files.