mirror of
https://github.com/Pungyeon/clean-go-article.git
synced 2024-11-10 09:24:04 +00:00
920 lines
42 KiB
Markdown
920 lines
42 KiB
Markdown
---
|
|
title: Clean Golang Code
|
|
output: pdf
|
|
---
|
|
|
|
# Clean Golang Code
|
|
|
|
## Abstract
|
|
Clean Code, is the pragmatic concept of ensuring readable and maintanable code. Clean Code establishes trust in the codebase and will steer developers away from introducing bugs. Clean Code will also establish much more stability in development speed, which typically will take a nose dive in the later stages of projects, due to higher risk of increasing bugs when introducing changes, as the codebase expands.
|
|
|
|
The document will start with a simple and short introduction of the fundamentals behind writing clean code and will thereafter transition into concrete refactoring examples. The aim of the document is to deliver the message of how easy it is to write clean code and how easy is it to write code, when it's clean.
|
|
|
|
## Context
|
|
* [Introduction to Clean Code](#Introduction-to-Clean-Code)
|
|
* Test Driven Development
|
|
* Cleaning Functions
|
|
* Variable Scope
|
|
|
|
* [Clean Golang](#Clean-Golang)
|
|
* Returning Defined Errors
|
|
* Returning Dynamic Errors
|
|
* Interfaces in Go
|
|
* The empty `interface{}`
|
|
* [Go Code Generation](#Go-Code-Generation)
|
|
* [Balancing Performance with Cleanliness](#Balancing-Performance-with-Cleanliness)
|
|
* Immutability - Variable Scope Continue
|
|
* [Index](#Index)
|
|
|
|
### Introduction to Clean Code
|
|
Clean Code, is the pragmatic concept of ensuring readable and maintanable code. Clean Code establishes trust in the codebase and will steer developers away from introducing bugs. Clean Code will also establish much more stability in development speed, which typically will take a nose dive in the later stages of projects, due to higher risk of increasing bugs when introducing changes, as the codebase expands.
|
|
|
|
#### Test Driven Development
|
|
The core of creating clean code stems from creating good tests. Writing good tests helps create clean code, as it invites developers to think about the outcomes and test coverage of functions / functionality. It's easier to test a function that is only 4 lines, rather than a function, which is 40. In the same manner, a function which is 4 lines, is typically easier to understand than a function of 40 lines. Therefore, when using test driven development, the resulting code is much more likely to be of a cleaner nature.
|
|
|
|
The next important part of test driven development, which is very closely related to clean code, is the TDD cycle:
|
|
|
|
1. Write a test which fails
|
|
2. Make the test pass
|
|
3. Refactor code
|
|
4. Repeat
|
|
|
|
Step three of the cycle, ensures that we can refactor our code as we are writing it. The tests ensure that our refactor doesn't change the outcome of our functions and we can therefore, essentially, go crazy refactoring our code to be as clean as possible. As we go along, and our codebase expands, we will still have our tests, to make sure that our refactoring will not affect the outcome of our functions.
|
|
|
|
#### Cleaning Functions
|
|
In the words of Robert C. Martin:
|
|
|
|
> "How small should a function be? Smaller than that!"
|
|
|
|
When writing clean code, our primary goal is to make our code easily digestable. The most effective way to do this, is to make our functions as small as possible. It's important to understand, that this is not to avoid code duplication, the actual reason for this is to heighten the code comprehension. Another way of explaining this, is to look at a function description:
|
|
|
|
```
|
|
fn GetItem:
|
|
- parse json input for order id
|
|
- get user from context
|
|
- check user has appropriate role
|
|
- get order from database
|
|
```
|
|
|
|
When using small functions (typically 5-8 lines in Golang), we can create a function that reads almost as easily as our description:
|
|
|
|
```go
|
|
var (
|
|
NullItem = Item{}
|
|
ErrInsufficientPrivliges = errors.New("user does not have sufficient priviliges")
|
|
)
|
|
|
|
func GetItem(ctx context.Context, json []bytes) (Item, error) {
|
|
order, err := NewItemFromJSON(json)
|
|
if err != nil {
|
|
return NullItem, err
|
|
}
|
|
if GetUserFromContext(ctx).IsAdmin() {
|
|
return db.GetItem(order.ID)
|
|
}
|
|
return NullItem, ErrInsufficientPrivliges
|
|
}
|
|
```
|
|
|
|
Using smaller functions also has a side-effect of eliminating another horrible habit of writing code: indentation hell. Indentation hell, typically occurs when a chain of if statements are clumsily inserted into a function. This makes the code very, very difficult to parse (for human beings) and should be eliminated whenever spotted. This is particularly common when working with `interface{}` and using type casting:
|
|
|
|
```go
|
|
func GetItemIfActive(extension string) (Item, error) {
|
|
if refIface, ok := db.ReferenceCache.Get(extension); ok {
|
|
if ref, ok := refIface.(string); ok {
|
|
if itemIface, ok := db.ItemCache.Get(ref); ok {
|
|
if item, ok := itemIface.(Item); ok {
|
|
if item.Active {
|
|
return Item, nil
|
|
} // return no item active
|
|
} // return cast error on item interface
|
|
} // return no item found in cache by reference
|
|
} // return cast error on reference
|
|
}
|
|
return EmptyItem, errors.New("reference not found in cache")
|
|
}
|
|
```
|
|
|
|
Not only, can this kind of code result in a very bad IDE experience for some programmers (due to line length), it's also almost impossible to track the flow of the code. It has to be noted, that go is not particularly likely to produce this kind of code, compared to languages with `try` `catch` blocks or typical callback implementations, but this still should be avoided at all costs. Please, keep in mind that the code above is a small example, with very simple code. As soon as it becomes more complex, the sphagetti factor of the code, grows exponentially.
|
|
|
|
So, how do we clean this? Well, it's actually quite simple. The first iteration, is to ensure that we are returning an error as soon as we can. Once we are done with this, we can split up our function into smaller functions as mentioned previously:
|
|
|
|
```go
|
|
func GetItemIfActive(extension string) (Item, error) {
|
|
refIface, ok := db.ReferenceCache.Get(extension)
|
|
if !ok {
|
|
return EmptyItem, errors.New("reference not found in cache")
|
|
}
|
|
|
|
if ref, ok := refIface.(string); ok {
|
|
// return cast error on reference
|
|
}
|
|
|
|
if itemIface, ok := db.ItemCache.Get(ref); ok {
|
|
// return no item found in cache by reference
|
|
}
|
|
|
|
if item, ok := itemIface.(Item); ok {
|
|
// return cast error on item interface
|
|
}
|
|
|
|
if !item.Active {
|
|
// return no item active
|
|
}
|
|
|
|
return Item, nil
|
|
}
|
|
```
|
|
|
|
And on second iteration, we will get something similar to the following:
|
|
|
|
```go
|
|
func GetItemIfActive(extension string) (Item, error) {
|
|
if ref, ok := getReference(extension) {
|
|
// return cast error on reference
|
|
}
|
|
return getActiveItemByReference(ref)
|
|
}
|
|
|
|
func getItemByReference(extension string) (string, bool) {
|
|
refIface, ok := db.ReferenceCache.Get(extension)
|
|
if !ok {
|
|
return EmptyItem, false
|
|
}
|
|
return refIface.(string)
|
|
}
|
|
|
|
func getActiveItemByReference(reference string) (Item, ) {
|
|
item, ok := getItemByReference(reference)
|
|
if !item.Active || !ok {
|
|
return EmptyItem, false
|
|
}
|
|
return Item, nil
|
|
}
|
|
|
|
func getItemByReference(reference string) (Item, bool) {
|
|
if itemIface, ok := db.ItemCache.Get(ref); ok {
|
|
return EmptyItem, false
|
|
}
|
|
return itemIface.(Item), true
|
|
}
|
|
```
|
|
> For production code, one should elaborate on the code even further, by returning errors instead of a `bool` values. This makes it much easier to understand where the error is originating from. However, as these are just example functinos, the `bool` values will suffice for now.
|
|
|
|
Now, this is many more lines of code than our first iteration. However, the code is so much easier to read. It's layered in an onion-style fashion, where we can ignore code that we aren't interested in knowing the details of and diving deeper into the functions that we wish to know the workings behind. When we do deep-dive into the lower level functionality, it will be extremely easy to comprehend, because we will only have to understand 3-5 lines in this case. This example illustrates, that we cannot score the cleaniless of our code from the line count of our functions. The first function iteration was much shorter. However, it was artificially short and very difficult to read. In most cases cleaning code will, to begin with, expand the already existing code base, in terms of lines of code. However, the benefit of readability is far preferred. If you are ever in doubt about this, think of how you feel about the following function, which does the same:
|
|
|
|
```go
|
|
func GetItemIfActive(extension string) (Item, error) {
|
|
if refIface,ok := db.ReferenceCache.Get(extension); ok {if ref,ok := refIface.(string); ok { if itemIface,ok := db.ItemCache.Get(ref); ok { if item,ok := itemIface.(Item); ok { if item.Active { return Item,nil }}}}} return EmptyItem, errors.New("reference not found in cache")
|
|
}
|
|
```
|
|
|
|
While we are on the topic. There are also a bunch of other side-effects that writing this style of code. Rather obviously, it makes our code much easier to test. It's much easier to get 100% code coverage on a function that is 4 lines (but written by a sane person), than a function which is 400 lines. That's common sense. However, this doesn't necessarily mean that people are willing to refactor their code and thereby make their lives easier. However, I advise, that if you are ever having difficulties with testing your code. Please consider refactoring your functions and trying again. It's most likely not: "because some things are just difficult to test", but rather that really large functions are just always difficult to test.
|
|
|
|
#### Variable Scope
|
|
Another nice side-effect of writing smaller functions. Is that it can typically eliminate using longer lasting mutable variables. Writing code with global variables, at least at a higher level, is a pratice of the past, it doesn't belong in clean code. Now, why is that? Well, the problem with using global variables is that we make it very difficult for programmers to understand the current state of a variable. If this variable is global and mutable, then, by definition, it's value can be changed by any other code in the codebase. At no point can you guarantee that this variable is going to be a specific value... This is a headache for everyone. But let's, look at a short example of how even larger scoped (not global) variables can cause problems. This is taken from an article named: [`Golang scope issue - A feature bug: Shadow Variables`](https://idiallo.com/blog/golang-scopes):
|
|
|
|
```go
|
|
func doComplex() (string, error) {
|
|
return "Success", nil
|
|
}
|
|
|
|
func main() {
|
|
var val string
|
|
num := 32
|
|
|
|
switch num {
|
|
case 16:
|
|
// do nothing
|
|
case 32:
|
|
val, err := doComplex()
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
if val == "" {
|
|
// do something else
|
|
}
|
|
case 64:
|
|
// do nothing
|
|
}
|
|
|
|
fmt.Println(val)
|
|
}
|
|
```
|
|
|
|
The problem with this code, from a quick skim, it seems like that the `var val string` value, should be printed out as: `Success` by the end of the `main` function. Unfortuantely, this is not the case. The reason for this is, the line:
|
|
|
|
> val, err := doComplex()
|
|
|
|
This declares a new variable `val` in the the switch case `32` scope and has nothing to do with the variable declared in the first line of `main`. Of course, it can be argued that the golang syntax is a little tricky, which I don't necessarily disagree with, but there is a much worse issue at hand. The declaration of `var val string` as a mutable largely scoped variable, is completely unecessary. If we do a **very** simple refactor, we will no longer have this issue:
|
|
|
|
```go
|
|
func getStringResult(num int) (string, error) {
|
|
switch num {
|
|
case 16:
|
|
// do nothing
|
|
case 32:
|
|
return doComplex()
|
|
case 64:
|
|
// do nothing
|
|
}
|
|
return ""
|
|
}
|
|
|
|
func main() {
|
|
val, err := getStringResult(32)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
if val == "" {
|
|
// do something else
|
|
}
|
|
fmt.Println(val)
|
|
}
|
|
```
|
|
|
|
Look at that, `val` never had to be mutated, nor did it have to have such a large scope. Again, keep in mind that these functions are very simple. Once this kind of code style becomes a part of larger more complex systems, it can be impossible to figure out, why errors are happening. We don't want this to happen. Not only because we generally dislike errors happening in software, but it is also disrespectful to our colleagues, and ourselves, that we are potentially wasting each others live's, having to debug this type of code. So, let's make a promise that, even though we disagree with golang declaration method, which isn't always the clearest, let's make sure that we never have to worry about it.
|
|
|
|
On a side not, if the `// do something else` part is another attempt to mutate the `val` variable. We should extract whatever logic in there as a function, as well as the previous part of it. This way, instead of prolonging the mutational scope of our variables, we can just return a new value:
|
|
|
|
```go
|
|
func getVal(num int) (string, error) {
|
|
val, err := getStringResult(32)
|
|
if err != nil {
|
|
return "", err
|
|
}
|
|
if val == "" {
|
|
return NewValue() // pretend function
|
|
}
|
|
}
|
|
|
|
func main() {
|
|
val, err := getVal(32)
|
|
if err != nil {
|
|
panic(err)
|
|
}
|
|
fmt.Println(val)
|
|
}
|
|
```
|
|
|
|
|
|
### Clean Golang
|
|
This section will describe some less generic aspects of writing clean golang code, but rather be discussing aspects that are very go specific. Like the previous section, there will still be a mix of generic and specific concepts being discussed, however, this section marks the start of the document, where the document changes from a generic description of clean code with golang examples, to golang specific descriptions, based on clean code principles.
|
|
|
|
|
|
#### Returning Defined Errors
|
|
We will be started out nice an easy, by describing a cleaner way to return errors. Like discussed earlier, our main goals with writing clean code, is to ensure readibility, testability and maintanability of the code base. This error returnign method will improve all three aspects, with very little effort.
|
|
|
|
Let's consider the normal way to return a custom error. This is a hypothetical example taken from a thread-safe map implementation, we have named `Store`:
|
|
|
|
```go
|
|
package smelly
|
|
|
|
func (store *Store) GetItem(id string) (Item, error) {
|
|
store.mtx.Lock()
|
|
defer store.mtx.Unlock()
|
|
|
|
item, ok := store.items[id]
|
|
if !ok {
|
|
return Item{}, errors.New("item could not be found in the store")
|
|
}
|
|
return item, nil
|
|
}
|
|
```
|
|
|
|
There is as such, nothing inherently smelly about this function, in its isolation. We look into the `items` map of our `Store` struct, to see if we already have an item with this `id`. If we do, we return the item, if we don't, we return an error. Pretty standard. So, what is the issue with returning custom errors like this? Well, let's look at what happens, when we use this function, from another package:
|
|
|
|
```go
|
|
func GetItemHandler(w http.ReponseWriter, r http.Request) {
|
|
item, err := smelly.GetItem("123")
|
|
if err != nil {
|
|
if err.Error() == "item could not be found in the store" {
|
|
http.Error(w, err.Error(), http.StatusNotFound)
|
|
return
|
|
}
|
|
http.Error(w, errr.Error(), http.StatusInternalServerError)
|
|
return
|
|
}
|
|
json.NewEncoder(w).Encode(item)
|
|
}
|
|
```
|
|
|
|
This is actually not too bad. However, there is one glaring problem with this. Errors in golang, are simply just an `interface` which implements a function (`Error()`) which returns a string. Therefore, we are now hardcoding the expected error code into our code base. This isn't too great. Mainly, because if we wish to change this error message to something else at another point, our code is too closely coupled, meaning that we would have to change our code in, possibly, many different places. Even worse, this could be a client using our package in their software, their software would inexplicably break all of a sudden after a package update. This is quite obviously somethign that we want to avoid. Fortunately, this fix is very simple.
|
|
|
|
```go
|
|
package clean
|
|
|
|
var (
|
|
NullItem = Item{}
|
|
|
|
ErrItemNotFound = errors.New("item could not be found in the store")
|
|
)
|
|
|
|
func (store *Store) GetItem(id string) (Item, error) {
|
|
store.mtx.Lock()
|
|
defer store.mtx.Unlock()
|
|
|
|
item, ok := store.items[id]
|
|
if !ok {
|
|
return NullItem, ErrItemNotFound
|
|
}
|
|
return item, nil
|
|
}
|
|
```
|
|
|
|
With this simple change of making the error into a variable `ErrItemNotFound`, we ensure that anyone using this package can check against the variable, rather than the actual string that it returns:
|
|
|
|
```go
|
|
func GetItemHandler(w http.ReponseWriter, r http.Request) {
|
|
item, err := clean.GetItem("123")
|
|
if err != nil {
|
|
if err == clean.ErrItemNotFound {
|
|
<(w, err.Error(), http.StatusNotFound)
|
|
return
|
|
}
|
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
|
return
|
|
}
|
|
json.NewEncoder(w).Encode(item)
|
|
}
|
|
```
|
|
|
|
This feels much nicer and is also much safer. Some would even say that it's easier to read as well. In the case of a more verbose error message, it certainly would be preferable for a developer to simply read `ErrItemNotFound` rather than a novel on why a certain error has been returned.
|
|
|
|
This approach is not limited to errors and can be used for other returned values. As an example, we are also returning a `NullItem` instead of `Item{}` as we did before. Again, this is more foolproof for future development and thereby also makes the code more maintainable. There can be many different scenarios in which it might be preferable to return this defined object, rather than initialising it on return. As an example, we could find out that we want to introduce more safety, in case developers using our package forget to check for errors and invoke a function, which now attempts to access a nil pointer reference. If we were to return the value on return, this could end up in an annoying code refactor. Whereas with our defined `Null` value, we just have to change a single line:
|
|
|
|
```go
|
|
var NullItem = Item{ pointerObj: NewPointerObj() }
|
|
```
|
|
|
|
#### Returning Dynamic Errors
|
|
There are certainly some scenarios, where returning an error variable might not actually be viable. In cases where customised errors' information is dynamic, to describe error events more specifically, we cannot define and return our static errors anymore. As an example:
|
|
|
|
```go
|
|
func (store *Store) GetItem(id string) (Item, error) {
|
|
store.mtx.Lock()
|
|
defer store.mtx.Unlock()
|
|
|
|
item, ok := store.items[id]
|
|
if !ok {
|
|
return NullItem, fmt.Errorf("Could not find item with ID: %s", id)
|
|
}
|
|
return item, nil
|
|
}
|
|
```
|
|
|
|
So, what to do? There is no well defined / standard method for handling and returning these kind of dynamic errors. My personal preference, is to return a new interface, with a bit of added functionality:
|
|
|
|
```go
|
|
type ErrorDetails interface {
|
|
Error() string
|
|
Type() string
|
|
}
|
|
|
|
type errDetails struct {
|
|
errtype error
|
|
details string
|
|
}
|
|
|
|
func NewErrorDetails(err error, details ...interface{}) ErrorDetails {
|
|
return &errDetails{
|
|
errtype: err,
|
|
details: details,
|
|
}
|
|
}
|
|
|
|
func (err *errDetails) Error() string {
|
|
return fmt.Sprintf("%v: %v", err.details)
|
|
}
|
|
|
|
func (err *errDetails) Type() error {
|
|
return err.errtype
|
|
}
|
|
```
|
|
|
|
This new data structure still works as our standard error struct. We can still compare it to `nil` since it's an interface implementation and we can still call `.Error()` on it, so it won't break any already existing implementations. However, the advantage is that we can now check our error type as we could previously, despite our error now containing the *dynamic* details:
|
|
|
|
```go
|
|
func (store *Store) GetItem(id string) (Item, error) {
|
|
store.mtx.Lock()
|
|
defer store.mtx.Unlock()
|
|
|
|
item, ok := store.items[id]
|
|
if !ok {
|
|
return NullItem, fmt.Errorf("Could not find item with ID: %s", id)
|
|
}
|
|
return item, nil
|
|
}
|
|
```
|
|
|
|
And our http handler function can then be refactored to check for a specific error again:
|
|
|
|
|
|
```go
|
|
func GetItemHandler(w http.ReponseWriter, r http.Request) {
|
|
item, err := clean.GetItem("123")
|
|
if err != nil {
|
|
if err.Type() == clean.ErrItemNotFound {
|
|
http.Error(w, err.Error(), http.StatusNotFound)
|
|
return
|
|
}
|
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
|
return
|
|
}
|
|
json.NewEncoder(w).Encode(item)
|
|
}
|
|
```
|
|
|
|
#### Interfaces in Go
|
|
In general, the go method for handling `interface`'s is quite different from other languages. Interfaces aren't explicitly implemented, like they would be in Java or C#, but are implicitly implemented, if they fulfill the contract of the interface. As an example, this means that any `struct` which has an `Error()` method, implements / fullfills the `Error` interface and can be returned as an error. This has it's advantages, as it makes golang feel more fast-paced and dynamic, as implementing an interface is extremely easy. The general proverb for writing golang functions is:
|
|
|
|
*"Be consdervative in what you do, be liberal in what you accept from others"*
|
|
|
|
In other words, you should write functions that accept an interface and return a concrete type. This is generally a good practice, and becomes super beneficial when doing tests with mocking. As an example, we can create a function which takes a writer interface as input and invokes the `Write` method of that inteface.
|
|
|
|
```go
|
|
type Pipe struct {
|
|
writer io.Writer
|
|
buffer bytes.Buffer
|
|
}
|
|
|
|
func NewPipe(w io.Writer) *Pipe {
|
|
return &Pipe{
|
|
writer: w,
|
|
}
|
|
}
|
|
|
|
func (pipe *Pipe) Save() error {
|
|
if _, err := pipe.writer.Write(pipe.FlushBuffer()); err != nil {
|
|
return err
|
|
}
|
|
return nil
|
|
}
|
|
```
|
|
|
|
Let's assume that we are writing to a file when our application is running, but we don't want to write to a new file for all tests which invokes this function. Therefore, we can implement a new mock type, which will basically do nothing. Essentially, this is just basic dependency injection and mocking, but the point is that it is extremely easy to use in go:
|
|
|
|
```go
|
|
type NullWriter struct {}
|
|
|
|
func (w *NullWriter) Write(b []byte) (int, error) {
|
|
return len(b), nil
|
|
}
|
|
|
|
func TestFn(t *testing.T) {
|
|
...
|
|
pipe := NewPipe(NullWriter{})
|
|
...
|
|
}
|
|
```
|
|
|
|
> NOTE: there is actually already a null writer implementation built into the ioutil package named `Discard`
|
|
|
|
When constructing our `Pipe` struct with the `NullWriter` (rather than a different writer), when invoking our `Save` function, nothing will happen. The only thing we had to do, was add 4 lines of code. This is why in idiomatic go, it is encouraged to make interface types as small as possible, to make implement a pattern like this as easy as possible. This is great and as I mentioned, is a great advantage of go. However, this implementation of interfaces, also comes with a *huge* downside.
|
|
|
|
|
|
### The empty `interface{}`
|
|
Unlike other languages, go does not have an implementation for generics. There have been many proposals on how to implement this and will eventually be implemented. However, without generics, developers are trying to find creative ways around this issue, very often using the empty `interface{}`. The next section, will describe why these, often too creative, implementations should be considered bad practice and unclean code. There will also be good examples of usage of the empty `interface{}` and how to avoid some pitfalls of writing code with the empty `interface{}`.
|
|
|
|
But first and foremost. What drives developers to use the empty `interface{}`? Well, as I said in the previously, the way that golang determines whether a concrete type implements an interface, is by checking whether it implements the methods of a specific interface. So what happens, if our interface implement no methods at all?
|
|
|
|
```go
|
|
type EmptyInterface interface {}
|
|
```
|
|
|
|
The above being equivalent to the built-in type `interface{}`. The result of this interface type is that **any** type is accepted. Meaning, that we can write functions in which any type is accepted. This is super useful for certain kind of functions, such as when creating a printer function. This is how it's possible to give any type to the `Println` function from the `fmt` package:
|
|
|
|
```go
|
|
func Println(v ...interface{}) {
|
|
...
|
|
}
|
|
```
|
|
|
|
In this case, we aren't only accepting a single `interface{}` but rather, a slice of types. These types can be of any type and can even be of different types, as long as they implement the empty `interface{}`, which funnily enough, they 100% will. This is a super common pattern when handling string conversation (both from and to string). The reason being, this is the only way in golang to implement generic methods. Good examples of this, come from the `json` standard library package:
|
|
|
|
```go
|
|
func InsertItemHandler(w http.ResponseWriter, r *http.Request) {
|
|
var item Item
|
|
if err := json.NewDecoder(r.Body).Decode(&item); err != nil {
|
|
http.Error(w, err.Error(), http.StatusBadRequest)
|
|
return
|
|
}
|
|
|
|
if err := db.InsertItem(item); err != nil {
|
|
http.Error(w, err.Error(), http.StatusInternalServerError)
|
|
return
|
|
}
|
|
w.WriteHeader(http.StatsOK)
|
|
}
|
|
```
|
|
|
|
The reason why this is a good way of using the empty `interface{}`. We are containing all the *messy* code inside our `Decode()` function, so that the developer using this functionality won't have to worry about reflection or casting of types. We just have to worry about providing a pointer to a concrete type. This is good, because the `Decode()` function is, essentially, returning a concrete type. However, even when using the empty `interface{}` with good practices, we still have some issues. If we pass a `JSON` string that has nothing to do with our `Item` type, we still won't receive an error. Out `item` variable will just be left with the default values. So, while we don't have to worry about reflection and casting errors, we will still have to make sure that the message sent from our client is a valid `Item` type. However, as of writing this document, there is no simple / good way to implement these type of generic decoders, without using the empty `interface{}` type.
|
|
|
|
The problem with this, is that we are leaning towards using golang (a statically typed languagae) as a dynamically typed langauge. This becomes even clearer, when looking at poor implementations of the `interface{}` type. The most common example of this, comes from developers trying to implement a generic store / list of some sort. Let's look at an example, trying to implement a generic HashMap package, which can store any type, using the `interface{}`.
|
|
|
|
```go
|
|
type HashMap struct {
|
|
store map[string]interface{}
|
|
}
|
|
|
|
func (hashmap *HashMap) Insert(key string, value interface{}) {
|
|
hashmap.store[key] = value
|
|
}
|
|
|
|
func (hashmap *HashMap) Get(id string) (interface{}, error) {
|
|
value, ok := hashmap.store[key]
|
|
if !ok {
|
|
return nil, ErrKeyNotFoundInHashMap
|
|
}
|
|
return value
|
|
}
|
|
```
|
|
|
|
> NOTE: I have omitted thread-safety from the example for simplicity
|
|
|
|
Please keep in mind that the implementation pattern used above, is used in quite a lot of golang packages. It is even used in the standard library `sync` package, for the `sync.Map` type. So, what is the big problem with this implementation? Well, let's have a look at an example of using this package.
|
|
|
|
```go
|
|
func SomeFunction(id string) (Item, error) {
|
|
itemIface, err := hashmap.Get(id)
|
|
if err != nil {
|
|
return EmptyItem, err
|
|
}
|
|
item, ok := itemIface.(Item)
|
|
if !ok {
|
|
return EmptyItem, ErrCastingItem
|
|
}
|
|
return item, nil
|
|
}
|
|
```
|
|
|
|
On first glance, this looks fine. However, like mentioned previously. What happens when the code base changes? As of right now, there is nothing limiting us from adding something other than the `Item` type. So what happens when someone starts adding other types into our HashMap? Our function now might return an error. This might even be a small change like someone else in the code base wanting to store a pointer `*Item` instead of an `Item`. Worst of all, this won't even be caught by our tests. We are simply checking that our function is working as expected and it does. The developer inserting the `*Item`, should of course add tests that will fail, but depending on the complexity of the system, might actually have a difficult time debugging why this test is failing.
|
|
|
|
This type of code, should never reach production. The matter of the fact is, that golang does not support generics as of now and as golang programmers, we should accept this. If we want to use generics, then we should use a different language which does support generics, rather than trying hack our way out of this. This is my biggest gripe with golang. A language that is otherwise quite good at forcing their idioms on their users, it baffles me that the use of `interface{}` is not more restricted. If anything, I feel that the return of an `interface{}` type should flag for a warning. I actually believe that it's that bad of a practice. If anything, it's much worse practice than using snake case instead of came case.
|
|
|
|
So, how do we prevent this code from reaching production? Well, let's borrow a principle from the Rust language. Generics are fully supported by Rust, which is actually quite strange, considering the language architecture. However, upon further insight, the way that Rust handles this, is very simple and very straight forward. Rust does not do runtime reflection, instead Rust does compiletime reflection. On compile, Rust will map out all the different possible variations of a generic function and compile a variant of this function, using concrete types rather than generic types. Referenced from: [this article](https://gist.github.com/Kimundi/8391398).
|
|
|
|
So, in other words rust is generating our functions for us. If we stick to golang code, that would be the equivalent of having, the following code:
|
|
|
|
```go
|
|
func SomeFunction(value interface{}) interface{} { ... }
|
|
|
|
func main() {
|
|
SomeFunction(42)
|
|
SomeFunction("hello there")
|
|
}
|
|
```
|
|
|
|
Which would generate the following code:
|
|
|
|
```go
|
|
func SomeFunction_Int(value int) int { ... }
|
|
func SomeFunction_String(value string) string { ... }
|
|
```
|
|
|
|
Unfortunately, there is no way in golang to directly translate this. However, we can actually get very very close to producing something similar with go code generation.
|
|
|
|
### Go Code Generation
|
|
Let me start out by saying, that go code generation is nothing fancy. It really, really isn't. At it's most basic, it's simply just templatling (much like HandleBars.js in JavaScript and Jinja in Python). However, it works really well, especially when it comes to writing generic code, that we don't want to write over and over again. Keep in mind, that it doesn't solve all of our problems and there are definitely situations in which actual generic lanauge support would be preferred. However, it is still greatly preferred to using empty `interface{}`
|
|
|
|
The following shows some basic go code generation, which will be able to create very simple, but thread-safe, HashMap for primitive types. This is an example of how to do this, and should give enough inspiration as how to expand on the example and create more elaborate code generation templates.
|
|
|
|
Our project has the following structure:
|
|
|
|
```
|
|
.
|
|
|-- generate
|
|
| `-- hashmap.go
|
|
|-- main.go
|
|
```
|
|
|
|
The `hashmap.go` file consists of a hashmap template and some logic, which will iterate over command line arguments to generate different types specified by the arguments. From the root of our project, we can run the following command:
|
|
|
|
> go run generate/hashmap.go int64 string uint16
|
|
|
|
This will generate files which implement a `HashMap` type for `int64` `string` and `uint16`. So the project now looks like the following:
|
|
|
|
```
|
|
.
|
|
|-- generate
|
|
| `-- main.go
|
|
|-- hashmap
|
|
| |-- hashmap_int64.go
|
|
| |-- hashmap_string.go
|
|
| `-- hashmap_uint16.go
|
|
|-- main.go
|
|
```
|
|
|
|
> NOTE: The full code can be found in the index of this document
|
|
|
|
We can now use these hashmap types in the rest of our codebase. These HashMaps are now no longer using `interface{}` but instead specifying concrete types for both input and output. As a short example, our get function now looks like this:
|
|
|
|
```go
|
|
func (hashmap *HashMap_int64) get(key string) (int64, error) {
|
|
value, ok := hashmap.store[key]
|
|
if !ok {
|
|
return Emptyint64, Errint64NotFoundInHashMap
|
|
}
|
|
return value
|
|
}
|
|
```
|
|
|
|
If we think about what was mentioned in the previous chapter, we can see why this is advantage for us. We are no longer having to do any type casting and we can be 100% sure that our functions are returning the correct concrete type. We can also be sure, that if breaking changes are introduced by changes the generated code, we will get compile time or test errors immediately. Not only does this make it easier to maintain our codebase, but it's also easier to understand the codebase and the effects of changing the codebase might have.
|
|
|
|
Of course, this doesn't come without certain disadvantages. We add a comment a top of every generated file:
|
|
|
|
> // Code generated by hashmap.go -Type int64 DO NOT EDIT.
|
|
|
|
This line, will tell certain IDE's to show a warning, that the file is not to be edited. However, nothing is actually preventing users from editing these files. Typically though, code generation is used along with a build script tool (such as a Makefile), so that all generated files are re-generated on build. This will, hopefully, prevent any users from making unwanted changes to the generated code. Of course, this also means that there is no room for tweaking our generated code type by type, but I feel this to be more of an advantage, than anything.
|
|
|
|
### Balancing Performance with Cleanliness
|
|
> TODO: I'm still not quite sure where to place this in the document
|
|
|
|
Everything comes with a price. Much like you *can't please all of the people all of the time*, you can't always have all your come code be 100% clean. In certain scenarios where you're main goal is to ensure the absolute most performant setup possible, it won't be possible to write the cleanest code. With everything else in life, comprimises are unavoidable and sometimes going against common best-practice can be a necessity. This is also absolutely fine, as long as we understand why the compromise was made.
|
|
|
|
Of course, if your main goal is to write top-performant applications. You're best bet, probably isn't golang. Golang is definitely a good choice within performance, but we shouldn't kid ourselves into believing that it can compete with the likes of C/C++ (or even Rust). That being said, it doesn't mean that we should all thoughts on creating performant code out of the window. This section will look at some concepts of ensuring clean code, while still ensuring a minimum loss of performance.
|
|
|
|
#### Immutability - (Variable Scope Continued)
|
|
Previously, we spoke about variable scope and how we should try to keep our variables immutable, as best possible. This is to avoid managing state, which is an immensely difficult part of programming and also to avoid confusion, when reading code. If state is changed in the flow of code, it can become extremely difficult to read and remember the current value of that state, making the code harder to debug. This becomes an even bigger problem when we start introducing concurrency into the picture. Functional programming languages pride themselves on being 100% concurrency safe, and rightly so! In functional programming all variables are immutable (at least to a certain extent). With this in mind, we can now guarantee that our code will never suffer from a race condition, as we will never mutate memory. That's neat!
|
|
|
|
Now, while rather controversial to mention in the golang language community. Golang, performance-wise, really benefits from borrowing a functional style. The performance bottleneck for golang, is the garbage collector. In most cases, this is why C/C++ and Rust outperform golang. Solving certain problems, however, where the computation is very reliant on fast memory allocation, golang performs ***horribly***. As an example of this (from: [The Computer Language Benchamrks Game](https://benchmarksgame-team.pages.debian.net/benchmarksgame/performance/binarytrees.html)), Rust is almost 8 times faster than go at creating binary trees. Even more insulting, there are Nodejs implementations that are faster than the fastest golang implementation.
|
|
|
|
It's really weird. But let's not think about it too much right now, because honestly, there is not much we *can* do about it. Instead, let's just agree that it's extremely unlikely that you will need to create binary trees performant in your day-to-day, so everything is probably going to be fine. The more important lesson we can learn from this, is that heap allocations should be avoided in golang code, if we want to keep it performant... and wow, what a coincidence... because this leads me to immutability and performance.
|
|
|
|
Let's take a look at a simple "FizzBuzz" implementation:
|
|
|
|
```go
|
|
func devnull(v ...interface{}) {
|
|
// don't print
|
|
}
|
|
|
|
func BenchmarkMutable(b *testing.B) {
|
|
buf := &bytes.Buffer{}
|
|
for i := 1; i < b.N; i++ {
|
|
buf.Reset()
|
|
|
|
if i%3 == 0 {
|
|
buf.WriteString("Fizz")
|
|
}
|
|
|
|
if i%5 == 0 {
|
|
buf.WriteString("Buzz")
|
|
}
|
|
|
|
if buf.String() == "" {
|
|
devnull(i)
|
|
} else {
|
|
devnull(buf.String())
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
Hey look it's a buffer implementation, and it get's worse, becasue the buf is a "global" variable, oh no! If we put the buf inside the for loop though, performance is absolutely terrible. Oh no ! What should we do? The allocation is tearing me apart, Lisa.
|
|
|
|
```go
|
|
func BenchmarkImmutable(b *testing.B) {
|
|
for i := 1; i < b.N; i++ {
|
|
devnull(
|
|
NewFizzBuzz(i).
|
|
AddIfDivisbleBy("Fizz", 3).
|
|
AddIfDivisbleBy("Buzz", 5).
|
|
Result(),
|
|
)
|
|
}
|
|
}
|
|
|
|
type FizzBuzz struct {
|
|
result string
|
|
number int
|
|
}
|
|
|
|
func NewFizzBuzz(n int) FizzBuzz {
|
|
return FizzBuzz{
|
|
number: n,
|
|
}
|
|
}
|
|
|
|
func (fb FizzBuzz) AddIfDivisbleBy(text string, divisor int) FizzBuzz {
|
|
if fb.number%divisor == 0 {
|
|
return FizzBuzz{result: fb.result + text, number: fb.number}
|
|
}
|
|
return fb
|
|
}
|
|
|
|
func (fb FizzBuzz) Result() interface{} {
|
|
if fb.result == "" {
|
|
return fb.number
|
|
}
|
|
return fb.result
|
|
}
|
|
```
|
|
|
|
Holy moly dude, that's the most complicated implementation of FizzBuzz I have ever seen in my life, that is some weird shit dude. But wow, 0 allocations.
|
|
|
|
```go
|
|
func BenchmarkThatsSoMutable(b *testing.B) {
|
|
for i := 1; i < b.N; i++ {
|
|
result := ""
|
|
if i%3 == 0 {
|
|
result += "Fizz"
|
|
}
|
|
if i%5 == 0 {
|
|
result += "Buzz"
|
|
}
|
|
if result != "" {
|
|
devnull(i)
|
|
} else {
|
|
devnull(result)
|
|
}
|
|
}
|
|
}
|
|
```
|
|
|
|
> NOTE: FizzBuzz implementation taken from article by Grayson Koonce, Engineering Manager at Uber: https://graysonkoonce.com/fizzbuzz-in-golang/
|
|
|
|
This solution is by no means awful and actually does quite well in our benchmark... However, there simply must be something wrong with these benchmarks.... I mean ffs... `result := ""` must surely be an allocation? Or is this all fucked, because it is put on the stack??? Wtf is going on here?
|
|
|
|
`TODO: don't have the time right now to finish this section. So I will just copy paste the code here, without any description`
|
|
|
|
## Index
|
|
|
|
### Generated Code
|
|
#### generate/hashmap.go
|
|
```go
|
|
package main
|
|
|
|
import (
|
|
"bytes"
|
|
"io/ioutil"
|
|
"os"
|
|
"path"
|
|
"strings"
|
|
"text/template"
|
|
)
|
|
|
|
type genValues struct {
|
|
Type string
|
|
}
|
|
|
|
func main() {
|
|
if err := clean(); err != nil {
|
|
panic(err)
|
|
}
|
|
|
|
if err := generate(os.Args[1:]); err != nil {
|
|
panic(err)
|
|
}
|
|
}
|
|
|
|
func clean() error {
|
|
if err := os.RemoveAll("hashmap"); err != nil {
|
|
if !os.IsNotExist(err) {
|
|
return err
|
|
}
|
|
}
|
|
return os.Mkdir("hashmap", 0755)
|
|
}
|
|
|
|
func generate(args []string) error {
|
|
tmpl, err := template.New("hashmap").Parse(hashmap)
|
|
if err != nil {
|
|
return err
|
|
}
|
|
for _, arg := range args {
|
|
if err := generateHashMap(arg, tmpl); err != nil {
|
|
return err
|
|
}
|
|
}
|
|
return nil
|
|
}
|
|
|
|
func generateHashMap(arg string, tmpl *template.Template) error {
|
|
buf := &bytes.Buffer{}
|
|
if err := tmpl.Execute(buf, genValues{Type: arg}); err != nil {
|
|
return err
|
|
}
|
|
return ioutil.WriteFile(createFilename(arg), buf.Bytes(), 0755)
|
|
}
|
|
|
|
func createFilename(arg string) string {
|
|
return path.Join(
|
|
"hashmap",
|
|
"hashmap_"+strings.ToLower(arg)+".go",
|
|
)
|
|
}
|
|
|
|
var hashmap = `// Code generated by hashmap.go -Type {{.Type}} DO NOT EDIT.
|
|
|
|
package hashmap
|
|
|
|
import (
|
|
"errors"
|
|
"sync"
|
|
)
|
|
|
|
var (
|
|
Empty{{.Type}} = {{.Type}}
|
|
|
|
Err{{.Type}}NotFoundInHashMap = errors.New("{{.Type}} was not found in hash map")
|
|
)
|
|
|
|
type HashMap_{{.Type}} struct {
|
|
store map[string]{{.Type}}
|
|
mtx sync.RWMutex
|
|
}
|
|
|
|
func NewHashMap_{{.Type}}() *HashMap_{{.Type}} {
|
|
return &HashMap_{{.Type}}{
|
|
store: map[string]{{.Type}}{},
|
|
}
|
|
}
|
|
|
|
func (hashmap *HashMap_{{.Type}}) Get(key string) ({{.Type}}, error) {
|
|
hashmap.mtx.RLock()
|
|
defer hashmap.mtx.RUnlock()
|
|
|
|
return hashmap.get(key)
|
|
}
|
|
|
|
func (hashmap *HashMap_{{.Type}}) get(key string) ({{.Type}}, error) {
|
|
value, ok := hashmap.store[key]
|
|
if !ok {
|
|
return Empty{{.Type}}, Err{{.Type}}NotFoundInHashMap
|
|
}
|
|
return value
|
|
}
|
|
|
|
func (hashmap *HashMap_{{.Type}}) Insert(key string, value {{.Type}}) {
|
|
hashmap.mtx.Lock()
|
|
defer hashmap.mtx.Unlock()
|
|
|
|
hashmap.store[key] = value
|
|
}
|
|
|
|
func (hashmap *HashMap_{{.Type}}) Delet(key string) {
|
|
hashmap.mtx.Lock()
|
|
defer hashmap.mtx.Unlock()
|
|
|
|
delete(key, hashmap.store)
|
|
}
|
|
`
|
|
```
|
|
|
|
#### hashmap/hashmap_int64.go
|
|
```go
|
|
// Code generated by hashmap.go -Type int64 DO NOT EDIT.
|
|
|
|
package hashmap
|
|
|
|
import (
|
|
"errors"
|
|
"sync"
|
|
)
|
|
|
|
var (
|
|
Emptyint64 = int64
|
|
|
|
Errint64NotFoundInHashMap = errors.New("int64 was not found in hash map")
|
|
)
|
|
|
|
type HashMap_int64 struct {
|
|
store map[string]int64
|
|
mtx sync.RWMutex
|
|
}
|
|
|
|
func NewHashMap_int64() *HashMap_int64 {
|
|
return &HashMap_int64{
|
|
store: map[string]int64{},
|
|
}
|
|
}
|
|
|
|
func (hashmap *HashMap_int64) Get(key string) (int64, error) {
|
|
hashmap.mtx.RLock()
|
|
defer hashmap.mtx.RUnlock()
|
|
|
|
return hashmap.get(key)
|
|
}
|
|
|
|
func (hashmap *HashMap_int64) get(key string) (int64, error) {
|
|
value, ok := hashmap.store[key]
|
|
if !ok {
|
|
return Emptyint64, Errint64NotFoundInHashMap
|
|
}
|
|
return value
|
|
}
|
|
|
|
func (hashmap *HashMap_int64) Insert(key string, value int64) {
|
|
hashmap.mtx.Lock()
|
|
defer hashmap.mtx.Unlock()
|
|
|
|
hashmap.store[key] = value
|
|
}
|
|
|
|
func (hashmap *HashMap_int64) Delet(key string) {
|
|
hashmap.mtx.Lock()
|
|
defer hashmap.mtx.Unlock()
|
|
|
|
delete(key, hashmap.store)
|
|
}
|
|
``` |