mirror of
https://github.com/Pungyeon/clean-go-article.git
synced 2024-11-23 14:14:05 +00:00
Edit up until Closures are Function Pointers
This commit is contained in:
AleksandrHovhannisyan
parent
1ade62d6fd
commit
3ffc571622
75
README.md
75
README.md
|
|
@ -145,7 +145,7 @@ func fileExtension(filepath string) string {
|
|||
}
|
||||
```
|
||||
|
||||
This kind of logical progression in our function names—from a high level of abstraction to a lower, more specific one&mdashmakes the code easier to follow and and read. Consider the alternative: If our highest level of abstraction is too specific, then we'll end up with a name that attempts to cover all bases, like `DetermineFileExtensionAndParseConfigurationFile`. This is horrendously difficult to read; we are trying to be too specific too soon and end up confusing the reader, despite trying to be clear!
|
||||
This kind of logical progression in our function names—from a high level of abstraction to a lower, more specific one—makes the code easier to follow and and read. Consider the alternative: If our highest level of abstraction is too specific, then we'll end up with a name that attempts to cover all bases, like `DetermineFileExtensionAndParseConfigurationFile`. This is horrendously difficult to read; we are trying to be too specific too soon and end up confusing the reader, despite trying to be clear!
|
||||
|
||||
#### Variable Naming
|
||||
Rather interestingly, the opposite is true for variables. Unlike functions, our variables should be named from more to less specific the deeper we go into nested scopes.
|
||||
|
|
@ -621,14 +621,14 @@ Looking at the example above, it's clear how this also simplifies the usage of o
|
|||
|
||||
## Clean Go
|
||||
|
||||
This section will describe some less generic aspects of writing clean Go 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 Go examples, to Go specific descriptions, based on clean code principles.
|
||||
This section will focus less on the generic aspects of writing clean Go code and more on the specifics, with an emphasis on the underlying clean code principles.
|
||||
|
||||
### Return Values
|
||||
|
||||
#### 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 readability, testability and maintainability of the code base. This error returning method will improve all three aspects, with very little effort.
|
||||
We'll start things off nice and easy by describing a cleaner way to return errors. As we discussed earlier, our main goal with writing clean code is to ensure readability, testability, and maintainability of the codebase. The technique for returning errors that we'll discuss here will achieve all three of those goals 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`:
|
||||
Let's consider the normal way to return a custom error. This is a hypothetical example taken from a thread-safe map implementation that we've named `Store`:
|
||||
|
||||
```go
|
||||
package smelly
|
||||
|
|
@ -645,7 +645,7 @@ func (store *Store) GetItem(id string) (Item, error) {
|
|||
}
|
||||
```
|
||||
|
||||
There is 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:
|
||||
There is nothing inherently smelly about this function when we consider it in isolation. We look into the `items` map of our `Store` struct to see if we already have an item with the given `id`. If we do, we return it; otherwise, we return an error. Pretty standard. So, what is the issue with returning custom errors as string values? Well, let's look at what happens when we use this function inside another package:
|
||||
|
||||
```go
|
||||
func GetItemHandler(w http.ReponseWriter, r http.Request) {
|
||||
|
|
@ -662,7 +662,9 @@ func GetItemHandler(w http.ReponseWriter, r http.Request) {
|
|||
}
|
||||
```
|
||||
|
||||
This is actually not too bad. However, there is one glaring problem with this. Errors in Go, 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 the error message value changes, our code breaks (softly). Our code is too closely coupled, meaning that we would have to change our code in, possibly, many different places. Even worse would be, if a client used our package to write this code. Their software would inexplicably break all of a sudden after a package update, should we choose to change the message of the returning error. This is quite obviously something that we want to avoid. Fortunately, the fix is very simple.
|
||||
This is actually not too bad. However, there is one glaring problem: An error in Go is simply an `interface` that implements a function (`Error()`) returning a string; thus, we are now hardcoding the expected error code into our codebase, which isn't ideal. This hardcoded string is known as a <strong>magic string</strong>. And its main problem is flexibility: If at some point we decide to change the string value used to represent an error, our code will break (softly) unless we update it in possibly many different places. Our code is tightly coupled—it relies on that specific magic string and the assumption that it will never change as the codebase grows.
|
||||
|
||||
An even worse situation would arise if a client were to use our package in their own code. Imagine that we decided to update our package and changed the string that represents an error—the client's software would now suddenly break. This is quite obviously something that we want to avoid. Fortunately, the fix is very simple:
|
||||
|
||||
```go
|
||||
package clean
|
||||
|
|
@ -685,7 +687,7 @@ func (store *Store) GetItem(id string) (Item, error) {
|
|||
}
|
||||
```
|
||||
|
||||
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:
|
||||
By simply representing the error as a variable (`ErrItemNotFound`), we've ensured 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) {
|
||||
|
|
@ -706,19 +708,21 @@ This feels much nicer and is also much safer. Some would even say that it's easi
|
|||
|
||||
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. There are many different scenarios in which it might be preferable to return a defined object, rather than initialising it on return.
|
||||
|
||||
Returning default `Null` values like the previous examples, can also be more safe, in certain cases. As an example, a user of our package could forget to check for errors and end up initialising a variable, pointing to an empty struct containing a default value of `nil` as one or more property values. When attempting to access this `nil` value later in the code, this could cause a panic in their code. However, when we return our custom default value instead, we can ensure that all values, which otherwise would default to `nil`, are initialised and thereby ensure that we do not cause panics in our users / clients software. This is also beneficial for ourselves, as if we wanted to achieve the same safety, without returning a default value, we would have to change our code, every place in which we return this type of empty value. However, with our default value approach, we now only have to change our code in a single place:
|
||||
Returning default `Null` values like we did in the previous examples can also be safer in certain cases. As an example, a user of our package could forget to check for errors and end up initialising a variable that points to an empty struct containing a default value of `nil` as one or more property values. When attempting to access this `nil` value later in the code, the client software would panic. However, when we return our custom default value instead, we can ensure that all values that would otherwise default to `nil` are initialised. Thus, we'd ensure that we do not cause panics in our users' software.
|
||||
|
||||
This also benefits us. Consider this: If we wanted to achieve the same safety without returning a default value, we would have to change our code everywhere we return this type of empty value. However, with our default value approach, we now only have to change our code in a single place:
|
||||
|
||||
```go
|
||||
var NullItem = Item{
|
||||
itemMap: map[string]Item{},
|
||||
}
|
||||
```
|
||||
> NOTE: In many scenarios, invoking the panic will actually be preferable. To indicate that there is an error check missing.
|
||||
> NOTE: In many scenarios, invoking a panic will actually be preferable to indicate that there is an error check missing.
|
||||
|
||||
> NOTE: Every interface property in Go, has a default value of `nil`. This means that this is useful, for any struct, which has an interface property. This is also true for structs which contain channels, maps and slices, which could potentially also have a `nil` value.
|
||||
> NOTE: Every interface property in Go has a default value of `nil`. This means that this is useful for any struct that has an interface property. This is also true for structs that contain channels, maps, and slices, which could potentially also have a `nil` value.
|
||||
|
||||
#### 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:
|
||||
There are certainly some scenarios where returning an error variable might not actually be viable. In cases where the information in customised errors is dynamic, if we want to describe error events more specifically, we can no longer define and return our static errors. Here's an example:
|
||||
|
||||
```go
|
||||
func (store *Store) GetItem(id string) (Item, error) {
|
||||
|
|
@ -733,7 +737,7 @@ func (store *Store) GetItem(id string) (Item, error) {
|
|||
}
|
||||
```
|
||||
|
||||
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:
|
||||
So, what to do? There is no well-defined or standard method for handling and returning these kinds of dynamic errors. My personal preference is to return a new interface, with a bit of added functionality:
|
||||
|
||||
```go
|
||||
type ErrorDetails interface {
|
||||
|
|
@ -762,7 +766,7 @@ func (err *errDetails) Type() error {
|
|||
}
|
||||
```
|
||||
|
||||
This new data structure still works as our standard error. 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:
|
||||
This new data structure still works as our standard error. 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 existing implementations. However, the advantage is that we can now check our error type as we could previously, despite our error now containing the <em>dynamic</em> details:
|
||||
|
||||
```go
|
||||
func (store *Store) GetItem(id string) (Item, error) {
|
||||
|
|
@ -777,8 +781,7 @@ func (store *Store) GetItem(id string) (Item, error) {
|
|||
}
|
||||
```
|
||||
|
||||
And our http handler function can then be refactored to check for a specific error again:
|
||||
|
||||
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) {
|
||||
|
|
@ -797,9 +800,9 @@ func GetItemHandler(w http.ReponseWriter, r http.Request) {
|
|||
|
||||
### Nil Values
|
||||
|
||||
A controversial aspect of Go, is the addition of `nil`. This value corresponds to the value `NULL` in C and is essentially an uninitialised pointer. Previously, we traversed in explained the troubles `nil` can cause, but to sum up: Things break, when you try to access methods or properties of a `nil` value. In the mentioned section, it was recommended to try an minimise usage of returning a `nil` value. This way, users of our code, would be less prone to accidentally access `nil` values by a mistake.
|
||||
A controversial aspect of Go is the addition of `nil`. This value corresponds to the value `NULL` in C and is essentially an uninitialised pointer. We've already seen some of the problems that `nil` can cause, but to sum up: Things break when you try to access methods or properties of a `nil` value. Thus, it's recommended to avoid returning a `nil` value when possible. This way, the users of our code are less likely to accidentally access `nil` values.
|
||||
|
||||
There are other scenarios in which it is common to find `nil` values, which can cause some unnecessary pain. As an example, the incorrect initialisation of a `struct` can lead to the `struct` containing `nil` properties. If accessed, they will cause a panic. An example of this, can be seen below:
|
||||
There are other scenarios in which it is common to find `nil` values that can cause some unnecessary pain. As an example, the incorrect initialisation of a `struct` can lead to the `struct` containing `nil` properties. If accessed, they will cause a panic. An example of this can be seen below:
|
||||
|
||||
```go
|
||||
type App struct {
|
||||
|
|
@ -819,18 +822,18 @@ func (cache *KVCache) Add(key, value string) {
|
|||
}
|
||||
```
|
||||
|
||||
This code is absolutely fine. However, we are exposed by the fact that our `App` can be initialised incorrectly, without initialising our `Cache` property within. Should the following code be invoked, our application will panic:
|
||||
This code is absolutely fine. However, the danger is that our `App` can be initialised incorrectly, without initialising the `Cache` property within. Should the following code be invoked, our application will panic:
|
||||
|
||||
```go
|
||||
app := App{}
|
||||
app.Cache.Add("panic", "now")
|
||||
```
|
||||
|
||||
The `Cache` property, has never been initialised and is therefore a `nil` pointer the `Add` method, is invoked. Running this code will result in a panic, with the following message:
|
||||
The `Cache` property, has never been initialised and is therefore a `nil` pointer. Thus, invoking the `Add` method like we did here will cause a panic, with the following message:
|
||||
|
||||
> panic: runtime error: invalid memory address or nil pointer dereference
|
||||
|
||||
Instead, we can turn our `Cache` property of our `App` structure into a private property and create a getter-like method, to access the `Cache` property of our `App`. This gives us more control of what we are returning and ensuring that we aren't returning a `nil` value.
|
||||
Instead, we can turn the `Cache` property of our `App` structure into a private property and create a getter-like method to access it. This gives us more control over what we are returning; specifically, it ensures that we aren't returning a `nil` value:
|
||||
|
||||
```go
|
||||
type App struct {
|
||||
|
|
@ -845,24 +848,24 @@ func (app *App) Cache() *KVCache {
|
|||
}
|
||||
```
|
||||
|
||||
We now ensure that we will never experience returning a `nil` pointer, when trying to access the `Cache` property. Our code, which previously panicked, will now be refactored to the following:
|
||||
The code that previously panicked will now be refactored to the following:
|
||||
|
||||
```go
|
||||
app := App{}
|
||||
app.Cache().Add("panic", "now")
|
||||
```
|
||||
|
||||
The reason why this is preferable, is that we are ensuring that users of our package aren't worrying about the implementation and whether they are using our package in an unsafe manner. All they need to worry about is writing their own clean code.
|
||||
This ensures that users of our package don't have to worry about the implementation and whether they're using our package in an unsafe manner. All they need to worry about is writing their own clean code.
|
||||
|
||||
> NOTE: There are other methods to achieve a similar safe outcome, however, I think that this is the most straightforward method of doing this.
|
||||
> NOTE: There are other methods of achieving a similarly safe outcome. However, I believe this is the most straightforward approach.
|
||||
|
||||
### Pointers in Go
|
||||
Pointers in go are rather a large topic. They are a very big part of working with the language, so much so, that it is essentially impossible to write go, without some knowledge of pointers and their workings in go. I will not go into detail, of the inner workings of Pointers in go in this article. Instead, we will focus on their quirks and how to handle them in go.
|
||||
Pointers in Go are a rather extensive topic. They're a very big part of working with the language—so much so that it is essentially impossible to write Go without some knowledge of pointers and their workings in the language. Therefore, it is important to understand how to use pointers without adding unnecessary complexity (and thereby keeping your codebase clean). Note that we will not review the details of how pointers are implemented in Go. Instead, we will focus on the quirks of Go pointers and how we can handle them.
|
||||
|
||||
Pointers add complexity, however, as mentioned, it's almost impossible to avoid them when writing go. Therefore, it is important to understand how to use pointers, without adding unnecessary complexity and thereby keeping your codebase clean. Without restraining oneself, the incorrect use of pointers can introduce nasty side-effects, introducing bugs that are particularly difficult to debug. Of course, when sticking to the basic principles of writing clean code, introduced in the first part of this article, we limit our exposure of introducing this complexity, but pointers are a particular case, which can still undo all of our previous hard work, of making our code clean.
|
||||
Pointers add complexity to code. If we aren't cautious, incorrectly using pointers can introduce nasty side effects or bugs that are particularly difficult to debug. By sticking to the basic principles of writing clean code that we covered in the first part of this document, we can at least reduce the chances of introducing unnecessary complexity to our code.
|
||||
|
||||
#### Pointer Mutability
|
||||
I have already used the word mutability more than once in this article, as a negative. Mutability is obviously not a clear-cut bad thing and I am by no means an advocate for writing 100% pure functional programs. Mutability is a powerful tool, but we should really only ever use it, when it's necessary. Let's have a look at a code example illustrating why:
|
||||
We've already looked at the problem of mutability in the context of globally or largely scoped variables. However, mutability is not necessarily always a bad thing, and I am by no means an advocate for writing 100% pure functional programs. Mutability is a powerful tool but we should really only ever use it when it's necessary. Let's have a look at a code example illustrating why:
|
||||
|
||||
```go
|
||||
func (store *UserStore) Insert(user *User) error {
|
||||
|
|
@ -879,7 +882,7 @@ func (store *UserStore) userExists(id int64) bool {
|
|||
}
|
||||
```
|
||||
|
||||
At first glance, this doesn't seem too bad. In fact, it might even seem like a rather simple insert function for a common list structure. We accept a pointer as input and if no other users with this `id` exist, then we insert the user pointer into our list. Now, we use this functionality in our public API for creating new users:
|
||||
At first glance, this doesn't seem too bad. In fact, it might even seem like a rather simple insert function for a common list structure. We accept a pointer as input, and if no other users with this `id` exist, then we insert the user pointer into our list. Then, we use this functionality in our public API for creating new users:
|
||||
|
||||
```go
|
||||
func CreateUser(w http.ResponseWriter, r *http.Request) {
|
||||
|
|
@ -903,13 +906,13 @@ func insertUser(w http.ResponseWriter, user User) error {
|
|||
}
|
||||
```
|
||||
|
||||
Once again, at first glance everything looks fine. We parse the user from the received request and insert the user struct into our store. Once we have inserted our user into the store successfully, we then set the password to nothing, before returning the user as a JSON object to our client. This is all quite common practice, typically when returning a user object, where the password has been hashed, we don't want to return the hashed password.
|
||||
Once again, at first glance, everything looks fine. We parse the user from the received request and insert the user struct into our store. Once we have successfully inserted our user into the store, we then set the password to be an empty string before returning the user as a JSON object to our client. This is all quite common practice, typically when returning a user object whose password has been hashed, since we don't want to return the hashed password.
|
||||
|
||||
However, imagine that we are using an in-memory store, based on a `map`, this code will produce some unexpected results. If we check in our user store, see that the change we made to the users password in the http handler function, also affected the object in our store. This is because the pointer address returned by `parseUserFromRequest`, is what we populated our store with, rather than an actual value. Therefore, when making changes to the dereferenced password value, we end up changing the value of the object we are pointing to in our store.
|
||||
However, imagine that we are using an in-memory store based on a `map`. This code will produce some unexpected results. If we check our user store, we'll see that the change we made to the users password in the HTTP handler function also affected the object in our store. This is because the pointer address returned by `parseUserFromRequest` is what we populated our store with, rather than an actual value. Therefore, when making changes to the dereferenced password value, we end up changing the value of the object we are pointing to in our store.
|
||||
|
||||
This is a great example of why both mutability and variable scope, can cause some serious issues and bugs, when used incorrectly. When passing pointers as an input parameter of a function, we are expanding the scope of our variable. Even more worrying, we are expanding the scope to an undefined level. We are *almost* expanding the scope of the variable to being a globally available variable. Depending on the variable scope of our store. As demonstrated by the above example, this can lead to disastrous bugs, which are particularly difficult to find and eradicate.
|
||||
This is a great example of why both mutability and variable scope can cause some serious issues and bugs when used incorrectly. When passing pointers as an input parameter of a function, we are expanding the scope of our variable. Even more worrying is the fact that we are expanding the scope to an undefined level. We are *almost* expanding the scope of the variable to the global level. As demonstrated by the above example, this can lead to disastrous bugs that are particularly difficult to find and eradicate.
|
||||
|
||||
Fortunately, the fix for this bug, is rather simple:
|
||||
Fortunately, the fix for this bug is rather simple:
|
||||
|
||||
```go
|
||||
func (store *UserStore) Insert(user User) error {
|
||||
|
|
@ -921,7 +924,7 @@ func (store *UserStore) Insert(user User) error {
|
|||
}
|
||||
```
|
||||
|
||||
Instead of passing a pointer to a `User` struct, we are now passing in a copy of a `User`. We are still storing a pointer to our store, however, instead of storing the pointer from outside of the function, we are storing the pointer to the copied value, which scope is inside the function. This fixes the immediate problem, but might still cause issues further down the line, if we aren't careful.
|
||||
Instead of passing a pointer to a `User` struct, we are now passing in a copy of a `User`. We are still storing a pointer to our store; however, instead of storing the pointer from outside of the function, we are storing the pointer to the copied value, whose scope is inside the function. This fixes the immediate problem but might still cause issues further down the line if we aren't careful. Consider this code:
|
||||
|
||||
```go
|
||||
func (store *UserStore) Get(id int64) (*User, error) {
|
||||
|
|
@ -933,11 +936,11 @@ func (store *UserStore) Get(id int64) (*User, error) {
|
|||
}
|
||||
```
|
||||
|
||||
Again, a very standard very simple implementation of a getter function for our store. However, this is still bad. We are once again expanding the scope of our pointer, which may end up causing unexpected side-effects. When returning the actual pointer value, which we are storing in our user store, we are essentially giving other parts of our application the ability to change our store values. This is bad, because it's bound to ensure confusion. Our store should be the only entity enabled to make changes to the values stored there. The easiest fix available for this, is to either return a value of `User` rather than returning a pointer.
|
||||
Again, this is a very standard implementation of a getter function for our store. However, it's still bad code because we are once again expanding the scope of our pointer, which may end up causing unexpected side-effects. When returning the actual pointer value, which we are storing in our user store, we are essentially giving other parts of our application the ability to change our store values. This is bound to cause confusion. Our store should be the only entity allowed to make changes to the values stored there. The easiest fix for this is to return a value of `User` rather than returning a pointer.
|
||||
|
||||
> NOTE: Should our application use multiple threads, which is often the case. Passing pointers to the same memory location, can also potentially result in a race condition. In other words, we aren't only potentially corrupting our data, we could also cause a panic from a data race.
|
||||
> NOTE: Consider the case where our application uses multiple threads. In this scenario, passing pointers to the same memory location can also potentially result in a race condition. In other words, we aren't only potentially corrupting our data—we could also cause a panic from a data race.
|
||||
|
||||
Please keep in mind, that there is intrinsically nothing wrong with returning pointers, however, the expanded scope and number of owners of the variables is the important aspect. This is what categorises our previous example a smelly operation. This is also why, that common Go constructors are also absolutely fine:
|
||||
Please keep in mind that there is intrinsically nothing wrong with returning pointers. However, the expanded scope of variables (and the number of owners that point to them) is the most important consideration when working with pointers. This is what categorises our previous example as a smelly operation. This is also why common Go constructors are also absolutely fine:
|
||||
|
||||
```go
|
||||
func AddName(user *User, name string) {
|
||||
|
|
@ -945,7 +948,7 @@ func AddName(user *User, name string) {
|
|||
}
|
||||
```
|
||||
|
||||
The reason why this is *ok*, is that the variable scope, which is defined by whomever invokes the functions, remains the same after the function returns. This combined with the face that the ownership of the variable remains unchanged (it stays solely with the function invoker), means that the pointer cannot be manipulated in an unexpected manner.
|
||||
This is *okay* because the variable scope, which is defined by whoever invokes the function, remains the same after the function returns. Combined with the fact that the ownership of the variable remains unchanged (it stays solely with the function invoker), this means that the pointer cannot be manipulated in an unexpected manner.
|
||||
|
||||
### Closures are Function Pointers
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue