Clean Go Code (Part 1)

Alexis Moody - Dec 11 '19 - - Dev Community

For most of my full stack career I've been a Ruby/JS dev working in Rails and React. But over the past 10 months I've dived head first into Golang due to my startup's acquisition. I've always been a big believer in human readable code that follows SOLID principles but I found the Go community tended towards short namespacing and single files with loosely coupled functions. To help improve my understanding of Golang I've decided to document some handy techniques I've learned that help keep your code easy to read and performant. These techniques are in no way one size fits all, but they have helped me in maintaining old and new microservices used by a 200+ engineering team. I hope you find value from this series as well!

Converting Data From One Type To Another

Imagine you are maintaining a social, long form text, posting platform. And let's say you're putting together an API that ingests a filter for a post's status.

Your SQL schema would look something like this:

CREATE TYPE posts.post_status_type AS ENUM (
    'DRAFT',
    'PUBLISHED',
    'UNDER_REVIEW'
);

CREATE TABLE posts.posts (
    id bigint NOT NULL,
    author_id bigint NOT NULL,
    title character varying(24) NOT NULL,
    content text,
    status posts.post_status_type NOT NULL   
);
Enter fullscreen mode Exit fullscreen mode

Your database layer Go struct would look something like this:

package db

type DatabasePost struct {
    ID       *int64
    Title    *string
    Content  *string
    Status   *string
    AuthorID *int64    
}

type DatabasePostFilter struct {
    Title  *string
    Status []*string
}

func FetchAllPosts(
    ctx context.Context, 
    authorID string, 
    f *DatabasePostFilter,
) ([]*DatabasePost, error) {
   // Query the database by formatting SQL 
}
Enter fullscreen mode Exit fullscreen mode

And your api layer structs would look like:

package api_models

type ApiPostStatus int32 
const (
    ApiPostStatusDraft       ApiPostStatus = 0
    ApiPostStatusPublished   ApiPostStatus = 1
    ApiPostStatusUnderReview ApiPostStatus = 2
)

type ApiPost struct {
    ID       *int64
    Title    *string
    Text     *string
    Status   *ApiPostStatus
    AuthorID *int64
}

type GetPostsRequest struct {
    AuthorID *int64
    Filter   *GetPostsRequestFilter
}

type GetPostsRequestFilter struct {
    Title  *string
    Status []*ApiPostStatus
}
Enter fullscreen mode Exit fullscreen mode

You could have that filter be a single select, but for the sake of this example we'll make it a little more flexible by allowing multiple statuses at the same time. On the SQL side of this filter the single select would result in SQL like:

SELECT * FROM posts WHERE (posts.status = $1);
Enter fullscreen mode Exit fullscreen mode

where the multiple status filter would look like:

SELECT * FROM posts WHERE (posts.status = $1 OR posts.status = $2);
Enter fullscreen mode Exit fullscreen mode

Now that we have a firm understanding of our data let's take a stab at converting our API layer status into our database layer status in an API function.

package api

// import a database package that contains all of the functions related // to interacting with the database
import (
    "src/posts/db"
    "src/posts/api_models"
)

type PostsAPI struct {
    DB db.PostsDB
}

func (a *PostsAPI) GetPosts(
    ctx context.Context, 
    r GetPostsRequest,
) ([]*ApiPost, error) {    
    // Convert the ApiPostStatus type to a string type
    var dbStatusStrings []*string
    for _, s := range r.Filter.Status {
        if s == api_models.ApiPostStatusDraft {
            dbStatusStrings = append(dbStatusStrings, "DRAFT")
        }
        if s == api_models.ApiPostStatusPuplished {
            dbStatusStrings = append(dbStatusStrings, "PUBLISHED")
        }
        if s == api_models.ApiPostStatusUnderReview {
            dbStatusStrings = append(dbStatusStrings, "UNDER_REVIEW")
        }
    }

    // Add the converted type to the filter struct
    dbPostFilter := DatabasePostFilter{
        Title: &r.Filter.Title,
        Status: &dbStatusStrings,
    }

    // Make the db level fetch for all of the author's posts
    dbPosts, err := db.FetchAllPosts(r.AuthorID, dbPostFilter)
    if err != nil {
        return nil, err
    }

    // Convert db posts to api posts
    var posts []*ApiPost
    for _, dbPost := range dbPosts {
        apiPost := &api_models.ApiPost{
            ID: &dbPost.ID,
            Title: &dbPost.Title,
            Text: &dbPost.Text,
            AuthorID: &dbPost.AuthorID
        }
        if dbPost.Status == "DRAFT" {
            apiPost.Status = api_models.ApiPostStatusDraft
        }
        if dbPost.Status == "PUBLISHED" {
            apiPost.Status = api_models.ApiPostStatusPublished
        }
        if dbPost.Status == "UNDER_REVIEW" {
            apiPost.Status = api_models.ApiPostStatusUnderReview
        }

        posts = append(posts, apiPost)
    }

    return posts, nil
}
Enter fullscreen mode Exit fullscreen mode

So that works. But you've got 6 if statements, and each one bumps up your cyclomatic complexity.

Separate your concerns

What can we do to increase the readability of this function and decrease the cyclomatic complexity? Let's try separating the concerns by pulling these checks into small helper functions in a converter package.

package converters

import "src/posts/api_models"

func apiStatusToDBStatus(
    statuses []*api_models.ApiPostStatus
) []*string {
    var dbStatusStrings []*string
    for _, s := range r.Filter.Status {
        if s == api_models.ApiPostStatusDraft {
            dbStatusStrings = append(dbStatusStrings, "DRAFT")
        }
        if s == api_models.ApiPostStatusPuplished {
            dbStatusStrings = append(dbStatusStrings, "PUBLISHED")
        }
        if s == api_models.ApiPostStatusUnderReview {
            dbStatusStrings = append(dbStatusStrings, "UNDER_REVIEW")
        }
    }

    return dbStatusStrings
}

func dbStatusToApiStatus(status *string) *api_models.ApiPostStatus {
    if status == "DRAFT" {
        return api_models.ApiPostStatusDraft
    }
    if status == "PUBLISHED" {
        return api_models.ApiPostStatusPublished
    }
    if status == "UNDER_REVIEW" {
        return api_models.ApiPostStatusUnderReview
    }
}
Enter fullscreen mode Exit fullscreen mode

Then our api function would look like:

package api

// import a database package that contains all of the functions related // to interacting with the database
import (
    ...
    "src/posts/converters"
)

func (a *PostsAPI) GetPosts(
    ctx context.Context, 
    r GetPostsRequest,
) ([]*ApiPost, error) {    
    dbStatusStrings := converters.apiStatusToDBStatus(r.Filter.Status)

    // Add the converted type to the filter struct

    // Make the db level fetch for all of the author's posts

    // Convert db posts to api posts
    for _, dbPost := range dbPosts {
        apiPost.Status = converters.dbStatusToApiStatus(dbPost.Status)
    }

    // return converted posts
}
Enter fullscreen mode Exit fullscreen mode

You could probably also move the entire apiPost to dbPost conversion to a converter package function. But for the sake of this exercise we'll leave that alone.

Simplify the approach

This approach is fine and your cyclomatic complexity has definitely gone down. But you're still using a function that essentially maps one static value to another. To me the maintainability of this api would be significantly improved by creating a static map between the database enum strings and the api status type. By storing these values in a variable we reduce the memory we need to allocate for our application and make it easier to guard against bad data.

Let's take a look!

package converters

import ...

var ApiStatusMap map[api_models.ApiPostStatus]string := {
    api_models.ApiPostStatusDraft: "DRAFT",
    api_models.ApiPostStatusPublished: "PUBLISHED",
    api_models.ApiPostStatusUnderReview: "UNDER_REVIEW",
}

var DBStatusMap map[string]api_models.ApiPostStatus := {
    "DRAFT": api_models.ApiPostStatusDraft,
    "PUBLISHED": api_models.ApiPostStatusPublished,
    "UNDER_REVIEW": api_models.ApiPostStatusUnderReview,
}

func ApiStatusToDBStatus(
    statuses []*api_models.ApiPostStatus
) []*string {
    var dbStatusStrings []*string
    for _, s := range r.Filter.Status {
        if dbStatus, ok := ApiStatusMap[s]; ok {
            dbStatusStrings = append(dbStatusStrings, dbStatus)
        }
    }

    return dbStatusStrings
}

func DbStatusToApiStatus(status *string) *api_models.ApiPostStatus {
    return DBStatusMap[status]
}
Enter fullscreen mode Exit fullscreen mode

By defining static conversion maps we reduce the complexity of our code while also improving the maintainability. If a new status type is added we can add 2 new lines to our maps instead of upwards of 6 lines to 2 different functions. In fact, we could probably remove the DbStatusToApiStatus function and use the DBStatusMap in our api function like:

    var posts []*ApiPost
    for _, dbPost := range dbPosts {
        apiPost := &api_models.ApiPost{
            ID: &dbPost.ID,
            Title: &dbPost.Title,
            Text: &dbPost.Text,
            AuthorID: &dbPost.AuthorID
        }
        if apiStatus, ok := converters.DBStatusMap[dbPost.Status]; ok {
            apiPost.Status = apiStatus
        }

        posts = append(posts, apiPost)
    }
Enter fullscreen mode Exit fullscreen mode

I hope this technique helps you when converting custom types. Feedback is always welcome and be sure to look out for the next post in this series soon!

. . . . .
Terabox Video Player