IO and sorting

    In the previous chapter we continued iterating on our application by adding a new endpoint . Along the way we learned about how to deal with JSON, embedding types and routing.

    Our product owner is somewhat perturbed by the software losing the scores when the server was restarted. This is because our implementation of our store is in-memory. She is also not pleased that we didn’t interpret the /league endpoint should return the players ordered by the number of wins!

    1. // InMemoryPlayerStore.go
    2. package main
    4. func NewInMemoryPlayerStore() *InMemoryPlayerStore {
    5.     return &InMemoryPlayerStore{map[string]int{}}
    6. }
    8. type InMemoryPlayerStore struct {
    9.     store map[string]int
    10. }
    12. func (i *InMemoryPlayerStore) GetLeague() []Player {
    13.     var league []Player
    14.     for name, wins := range i.store {
    15.         league = append(league, Player{name, wins})
    16.     }
    17.     return league
    18. }
    20. func (i *InMemoryPlayerStore) RecordWin(name string) {
    21.     i.store[name]++
    22. }
    24. func (i *InMemoryPlayerStore) GetPlayerScore(name string) int {
    25.     return i.store[name]
    26. }
    1. // main.go
    2. package main
    4. import (
    5.     "log"
    6.     "net/http"
    7. )
    9. func main() {
    10.     server := NewPlayerServer(NewInMemoryPlayerStore())
    12.     if err := http.ListenAndServe(":5000", server); err != nil {
    13.         log.Fatalf("could not listen on port 5000 %v", err)
    14.     }
    15. }

    You can find the corresponding tests in the link at the top of the chapter.

    Store the data

    There are dozens of databases we could use for this but we’re going to go for a very simple approach. We’re going to store the data for this application in a file as JSON.

    This keeps the data very portable and is relatively simple to implement.

    It won’t scale especially well but given this is a prototype it’ll be fine for now. If our circumstances change and it’s no longer appropriate it’ll be simple to swap it out for something different because of the PlayerStore abstraction we have used.

    We will keep the InMemoryPlayerStore for now so that the integration tests keep passing as we develop our new store. Once we are confident our new implementation is sufficient to make the integration test pass we will swap it in and then delete InMemoryPlayerStore.

    Write the test first

    By now you should be familiar with the interfaces around the standard library for reading data (io.Reader), writing data (io.Writer) and how we can use the standard library to test these functions without having to use real files.

    For this work to be complete we’ll need to implement PlayerStore so we’ll write tests for our store calling the methods we need to implement. We’ll start with GetLeague.

    1. func TestFileSystemStore(t *testing.T) {
    3.     t.Run("/league from a reader", func(t *testing.T) {
    4.         database := strings.NewReader(`[
    5.             {"Name": "Cleo", "Wins": 10},
    6.             {"Name": "Chris", "Wins": 33}]`)
    8.         store := FileSystemPlayerStore{database}
    10.         got := store.GetLeague()
    12.         want := []Player{
    13.             {"Cleo", 10},
    14.             {"Chris", 33},
    15.         }
    17.         assertLeague(t, got, want)
    18.     })
    19. }

    We’re using strings.NewReader which will return us a Reader, which is what our FileSystemPlayerStore will use to read data. In main we will open a file, which is also a Reader.

    Try to run the test

    1. # github.com/quii/learn-go-with-tests/json-and-io/v7
    2. ./FileSystemStore_test.go:15:12: undefined: FileSystemPlayerStore

    Write the minimal amount of code for the test to run and check the failing test output

    Let’s define FileSystemPlayerStore in a new file

    1. type FileSystemPlayerStore struct {}

    Try again

    1. # github.com/quii/learn-go-with-tests/json-and-io/v7
    2. ./FileSystemStore_test.go:15:28: too many values in struct initializer
    3. ./FileSystemStore_test.go:17:15: store.GetLeague undefined (type FileSystemPlayerStore has no field or method GetLeague)

    It’s complaining because we’re passing in a Reader but not expecting one and it doesn’t have GetLeague defined yet.

    1. type FileSystemPlayerStore struct {
    2.     database io.Reader
    3. }
    5. func (f *FileSystemPlayerStore) GetLeague() []Player {
    6.     return nil
    7. }

    One more try…

    1. === RUN   TestFileSystemStore//league_from_a_reader
    2.     --- FAIL: TestFileSystemStore//league_from_a_reader (0.00s)
    3.         FileSystemStore_test.go:24: got [] want [{Cleo 10} {Chris 33}]

    Write enough code to make it pass

    We’ve read JSON from a reader before

    1. func (f *FileSystemPlayerStore) GetLeague() []Player {
    2.     var league []Player
    3.     json.NewDecoder(f.database).Decode(&league)
    4.     return league
    5. }

    The test should pass.

    Refactor

    We have done this before! Our test code for the server had to decode the JSON from the response.

    Let’s try DRYing this up into a function.

    Create a new file called league.go and put this inside.

    1. func NewLeague(rdr io.Reader) ([]Player, error) {
    2.     var league []Player
    3.     err := json.NewDecoder(rdr).Decode(&league)
    4.     if err != nil {
    5.         err = fmt.Errorf("problem parsing league, %v", err)
    6.     }
    8.     return league, err
    9. }

    Call this in our implementation and in our test helper getLeagueFromResponse in server_test.go

    1. func (f *FileSystemPlayerStore) GetLeague() []Player {
    2.     league, _ := NewLeague(f.database)
    3.     return league
    4. }

    We haven’t got a strategy yet for dealing with parsing errors but let’s press on.

    There is a flaw in our implementation. First of all, let’s remind ourselves how io.Reader is defined.

    1. type Reader interface {
    2.     Read(p []byte) (n int, err error)
    3. }

    With our file, you can imagine it reading through byte by byte until the end. What happens if you try and Read a second time?

    Add the following to the end of our current test.

    1. // read again
    2. got = store.GetLeague()
    3. assertLeague(t, got, want)

    We want this to pass, but if you run the test it doesn’t.

    The problem is our Reader has reached the end so there is nothing more to read. We need a way to tell it to go back to the start.

    is another interface in the standard library that can help.

    1. type ReadSeeker interface {
    2.     Reader
    3.     Seeker
    4. }

    Remember embedding? This is an interface comprised of Reader and Seeker

    1. type Seeker interface {
    2.     Seek(offset int64, whence int) (int64, error)
    3. }

    This sounds good, can we change FileSystemPlayerStore to take this interface instead?

    1. type FileSystemPlayerStore struct {
    2.     database io.ReadSeeker
    3. }
    5. func (f *FileSystemPlayerStore) GetLeague() []Player {
    6.     f.database.Seek(0, 0)
    7.     league, _ := NewLeague(f.database)
    8.     return league
    9. }

    Try running the test, it now passes! Happily for us string.NewReader that we used in our test also implements ReadSeeker so we didn’t have to make any other changes.

    Next we’ll implement GetPlayerScore.

    Write the test first

    1. t.Run("get player score", func(t *testing.T) {
    2.     database := strings.NewReader(`[
    3.         {"Name": "Cleo", "Wins": 10},
    4.         {"Name": "Chris", "Wins": 33}]`)
    6.     store := FileSystemPlayerStore{database}
    8.     got := store.GetPlayerScore("Chris")
    10.     want := 33
    12.     if got != want {
    13.         t.Errorf("got %d want %d", got, want)
    14.     }
    15. })

    Try to run the test

    ./FileSystemStore_test.go:38:15: store.GetPlayerScore undefined (type FileSystemPlayerStore has no field or method GetPlayerScore)

    Write the minimal amount of code for the test to run and check the failing test output

    We need to add the method to our new type to get the test to compile.

    1. func (f *FileSystemPlayerStore) GetPlayerScore(name string) int {
    2.     return 0
    3. }

    Now it compiles and the test fails

    1. === RUN   TestFileSystemStore/get_player_score
    2.     --- FAIL: TestFileSystemStore//get_player_score (0.00s)
    3.         FileSystemStore_test.go:43: got 0 want 33

    Write enough code to make it pass

    We can iterate over the league to find the player and return their score

    1. func (f *FileSystemPlayerStore) GetPlayerScore(name string) int {
    3.     var wins int
    5.     for _, player := range f.GetLeague() {
    6.         if player.Name == name {
    7.             wins = player.Wins
    8.             break
    9.         }
    10.     }
    12.     return wins
    13. }

    You will have seen dozens of test helper refactorings so I’ll leave this to you to make it work

    Finally, we need to start recording scores with RecordWin.

    Write the test first

    Our approach is fairly short-sighted for writes. We can’t (easily) just update one “row” of JSON in a file. We’ll need to store the whole new representation of our database on every write.

    How do we write? We’d normally use a Writer but we already have our ReadSeeker. Potentially we could have two dependencies but the standard library already has an interface for us ReadWriteSeeker which lets us do all the things we’ll need to do with a file.

    Let’s update our type

    1. type FileSystemPlayerStore struct {
    2.     database io.ReadWriteSeeker
    3. }

    See if it compiles

    1. ./FileSystemStore_test.go:15:34: cannot use database (type *strings.Reader) as type io.ReadWriteSeeker in field value:
    2.     *strings.Reader does not implement io.ReadWriteSeeker (missing Write method)
    3. ./FileSystemStore_test.go:36:34: cannot use database (type *strings.Reader) as type io.ReadWriteSeeker in field value:
    4.     *strings.Reader does not implement io.ReadWriteSeeker (missing Write method)

    We have two choices

    • Create a temporary file for each test. *os.File implements ReadWriteSeeker. The pro of this is it becomes more of an integration test, we’re really reading and writing from the file system so it will give us a very high level of confidence. The cons are we prefer unit tests because they are faster and generally simpler. We will also need to do more work around creating temporary files and then making sure they’re removed after the test.
    • We could use a third party library. Mattetti has written a library which implements the interface we need and doesn’t touch the file system.

    I don’t think there’s an especially wrong answer here, but by choosing to use a third party library I would have to explain dependency management! So we will use files instead.

    Before adding our test we need to make our other tests compile by replacing the strings.Reader with an os.File.

    Let’s create a helper function which will create a temporary file with some data inside it

    1. func createTempFile(t *testing.T, initialData string) (io.ReadWriteSeeker, func()) {
    2.     t.Helper()
    4.     tmpfile, err := ioutil.TempFile("", "db")
    6.     if err != nil {
    7.         t.Fatalf("could not create temp file %v", err)
    9.     tmpfile.Write([]byte(initialData))
    11.     removeFile := func() {
    12.         tmpfile.Close()
    13.         os.Remove(tmpfile.Name())
    14.     }
    16.     return tmpfile, removeFile
    17. }

    TempFile creates a temporary file for us to use. The "db" value we’ve passed in is a prefix put on a random file name it will create. This is to ensure it won’t clash with other files by accident.

    You’ll notice we’re not only returning our ReadWriteSeeker (the file) but also a function. We need to make sure that the file is removed once the test is finished. We don’t want to leak details of the files into the test as it’s prone to error and uninteresting for the reader. By returning a removeFile function, we can take care of the details in our helper and all the caller has to do is run defer cleanDatabase().

    1. func TestFileSystemStore(t *testing.T) {
    3.     t.Run("league from a reader", func(t *testing.T) {
    4.         database, cleanDatabase := createTempFile(t, `[
    5.             {"Name": "Cleo", "Wins": 10},
    6.             {"Name": "Chris", "Wins": 33}]`)
    7.         defer cleanDatabase()
    9.         store := FileSystemPlayerStore{database}
    11.         got := store.GetLeague()
    13.         want := []Player{
    15.             {"Chris", 33},
    16.         }
    18.         assertLeague(t, got, want)
    20.         // read again
    21.         got = store.GetLeague()
    22.         assertLeague(t, got, want)
    23.     })
    25.     t.Run("get player score", func(t *testing.T) {
    26.         database, cleanDatabase := createTempFile(t, `[
    27.             {"Name": "Cleo", "Wins": 10},
    28.             {"Name": "Chris", "Wins": 33}]`)
    29.         defer cleanDatabase()
    31.         store := FileSystemPlayerStore{database}
    33.         got := store.GetPlayerScore("Chris")
    34.         want := 33
    35.         assertScoreEquals(t, got, want)
    36.     })
    37. }

    Run the tests and they should be passing! There were a fair amount of changes but now it feels like we have our interface definition complete and it should be very easy to add new tests from now.

    Let’s get the first iteration of recording a win for an existing player

    1. t.Run("store wins for existing players", func(t *testing.T) {
    2.     database, cleanDatabase := createTempFile(t, `[
    3.         {"Name": "Cleo", "Wins": 10},
    4.         {"Name": "Chris", "Wins": 33}]`)
    5.     defer cleanDatabase()
    7.     store := FileSystemPlayerStore{database}
    9.     store.RecordWin("Chris")
    11.     got := store.GetPlayerScore("Chris")
    12.     want := 34
    13.     assertScoreEquals(t, got, want)
    14. })

    Try to run the test

    ./FileSystemStore_test.go:67:8: store.RecordWin undefined (type FileSystemPlayerStore has no field or method RecordWin)

    Write the minimal amount of code for the test to run and check the failing test output

    Add the new method

    1. func (f *FileSystemPlayerStore) RecordWin(name string) {
    3. }
    1. === RUN   TestFileSystemStore/store_wins_for_existing_players
    2.     --- FAIL: TestFileSystemStore/store_wins_for_existing_players (0.00s)
    3.         FileSystemStore_test.go:71: got 33 want 34

    Our implementation is empty so the old score is getting returned.

    Write enough code to make it pass

    1. func (f *FileSystemPlayerStore) RecordWin(name string) {
    2.     league := f.GetLeague()
    4.     for i, player := range league {
    5.         if player.Name == name {
    6.             league[i].Wins++
    7.         }
    8.     }
    10.     f.database.Seek(0,0)
    11.     json.NewEncoder(f.database).Encode(league)
    12. }

    You may be asking yourself why I am doing league[i].Wins++ rather than player.Wins++.

    When you range over a slice you are returned the current index of the loop (in our case i) and a copy of the element at that index. Changing the Wins value of a copy won’t have any effect on the league slice that we iterate on. For that reason, we need to get the reference to the actual value by doing league[i] and then changing that value instead.

    If you run the tests, they should now be passing.

    Refactor

    In GetPlayerScore and RecordWin, we are iterating over []Player to find a player by name.

    We could refactor this common code in the internals of FileSystemStore but to me, it feels like this is maybe useful code we can lift into a new type. Working with a “League” so far has always been with []Player but we can create a new type called League. This will be easier for other developers to understand and then we can attach useful methods onto that type for us to use.

    Inside league.go add the following

    1. type League []Player
    3. func (l League) Find(name string) *Player {
    4.     for i, p := range l {
    5.         if p.Name==name {
    6.             return &l[i]
    7.         }
    8.     }
    9.     return nil
    10. }

    Now if anyone has a League they can easily find a given player.

    Change our PlayerStore interface to return League rather than []Player. Try and re-run the tests, you’ll get a compilation problem because we’ve changed the interface but it’s very easy to fix; just change the return type from []Player to League.

    This lets us simplify our methods in FileSystemStore.

    1. func (f *FileSystemPlayerStore) GetPlayerScore(name string) int {
    3.     player := f.GetLeague().Find(name)
    5.     if player != nil {
    6.         return player.Wins
    7.     }
    9.     return 0
    10. }
    12. func (f *FileSystemPlayerStore) RecordWin(name string) {
    13.     league := f.GetLeague()
    14.     player := league.Find(name)
    16.     if player != nil {
    17.         player.Wins++
    18.     }
    20.     f.database.Seek(0, 0)
    21.     json.NewEncoder(f.database).Encode(league)
    22. }

    This is looking much better and we can see how we might be able to find other useful functionality around League can be refactored.

    We now need to handle the scenario of recording wins of new players.

    Write the test first

    1. t.Run("store wins for new players", func(t *testing.T) {
    2.     database, cleanDatabase := createTempFile(t, `[
    3.         {"Name": "Cleo", "Wins": 10},
    4.         {"Name": "Chris", "Wins": 33}]`)
    5.     defer cleanDatabase()
    7.     store := FileSystemPlayerStore{database}
    9.     store.RecordWin("Pepper")
    11.     got := store.GetPlayerScore("Pepper")
    12.     want := 1
    13.     assertScoreEquals(t, got, want)
    14. })

    Try to run the test

    1. === RUN   TestFileSystemStore/store_wins_for_new_players#01
    2.     --- FAIL: TestFileSystemStore/store_wins_for_new_players#01 (0.00s)
    3.         FileSystemStore_test.go:86: got 0 want 1

    Write enough code to make it pass

    We just need to handle the scenario where Find returns nil because it couldn’t find the player.

    1. func (f *FileSystemPlayerStore) RecordWin(name string) {
    2.     league := f.GetLeague()
    3.     player := league.Find(name)
    5.     if player != nil {
    6.         player.Wins++
    7.     } else {
    8.         league = append(league, Player{name, 1})
    9.     }
    11.     f.database.Seek(0, 0)
    12.     json.NewEncoder(f.database).Encode(league)
    13. }

    The happy path is looking ok so we can now try using our new Store in the integration test. This will give us more confidence that the software works and then we can delete the redundant InMemoryPlayerStore.

    In TestRecordingWinsAndRetrievingThem replace the old store.

    1. database, cleanDatabase := createTempFile(t, "")
    2. defer cleanDatabase()
    3. store := &FileSystemPlayerStore{database}

    If you run the test it should pass and now we can delete InMemoryPlayerStore. main.go will now have compilation problems which will motivate us to now use our new store in the “real” code.

    1. package main
    3. import (
    4.     "log"
    5.     "net/http"
    6.     "os"
    7. )
    9. const dbFileName = "game.db.json"
    11. func main() {
    12.     db, err := os.OpenFile(dbFileName, os.O_RDWR|os.O_CREATE, 0666)
    14.     if err != nil {
    15.         log.Fatalf("problem opening %s %v", dbFileName, err)
    16.     }
    18.     store := &FileSystemPlayerStore{db}
    19.     server := NewPlayerServer(store)
    21.     if err := http.ListenAndServe(":5000", server); err != nil {
    22.         log.Fatalf("could not listen on port 5000 %v", err)
    23.     }
    24. }
    • We create a file for our database.
    • The 2nd argument to os.OpenFile lets you define the permissions for opening the file, in our case O_RDWR means we want to read and write and os.O_CREATE means create the file if it doesn’t exist.
    • The 3rd argument means sets permissions for the file, in our case, all users can read and write the file. (See superuser.com for a more detailed explanation).

    Running the program now persists the data in a file in between restarts, hooray!

    More refactoring and performance concerns

    Every time someone calls GetLeague() or GetPlayerScore() we are reading the file from the start, and parsing it into JSON. We should not have to do that because FileSystemStore is entirely responsible for the state of the league; we just want to use the file at the start to get the current state and updating it when data changes.

    We can create a constructor which can do some of this initialisation for us and store the league as a value in our FileSystemStore to be used on the reads instead.

    1. type FileSystemPlayerStore struct {
    2.     database io.ReadWriteSeeker
    3.     league League
    4. }
    6. func NewFileSystemPlayerStore(database io.ReadWriteSeeker) *FileSystemPlayerStore {
    7.     database.Seek(0, 0)
    8.     league, _ := NewLeague(database)
    9.     return &FileSystemPlayerStore{
    10.         database:database,
    11.         league:league,
    12.     }
    13. }

    This way we only have to read from disk once. We can now replace all of our previous calls to getting the league from disk and just use f.league instead.

    1. func (f *FileSystemPlayerStore) GetLeague() League {
    2.     return f.league
    3. }
    5. func (f *FileSystemPlayerStore) GetPlayerScore(name string) int {
    7.     player := f.league.Find(name)
    9.     if player != nil {
    10.         return player.Wins
    11.     }
    13.     return 0
    14. }
    16. func (f *FileSystemPlayerStore) RecordWin(name string) {
    17.     player := f.league.Find(name)
    19.     if player != nil {
    20.         player.Wins++
    21.     } else {
    22.         f.league = append(f.league, Player{name, 1})
    23.     }
    25.     f.database.Seek(0, 0)
    26. }

    If you try and run the tests it will now complain about initialising FileSystemPlayerStore so just fix them by calling our new constructor.

    Another problem

    There is some more naivety in the way we are dealing with files which could create a very nasty bug down the line.

    When we RecordWin we Seek back to the start of the file and then write the new data but what if the new data was smaller than what was there before?

    In our current case, this is impossible. We never edit or delete scores so the data can only get bigger but it would be irresponsible for us to leave the code like this, it’s not unthinkable that a delete scenario could come up.

    How will we test for this though? What we need to do is first refactor our code so we separate out the concern of the kind of data we write, from the writing. We can then test that separately to check it works how we hope.

    We’ll create a new type to encapsulate our “when we write we go from the beginning” functionality. I’m going to call it Tape. Create a new file with the following

    1. package main
    3. import "io"
    5. type tape struct {
    7. }
    9. func (t *tape) Write(p []byte) (n int, err error) {
    10.     t.file.Seek(0, 0)
    11.     return t.file.Write(p)
    12. }

    Notice that we’re only implementing Write now, as it encapsulates the Seek part. This means our FileSystemStore can just have a reference to a Writer instead.

    1. type FileSystemPlayerStore struct {
    2.     database io.Writer
    3.     league   League
    4. }

    Update the constructor to use Tape

    1. func NewFileSystemPlayerStore(database io.ReadWriteSeeker) *FileSystemPlayerStore {
    2.     database.Seek(0, 0)
    3.     league, _ := NewLeague(database)
    5.     return &FileSystemPlayerStore{
    6.         database: &tape{database},
    7.         league:   league,
    8.     }
    9. }

    Finally, we can get the amazing payoff we wanted by removing the Seek call from RecordWin. Yes, it doesn’t feel much, but at least it means if we do any other kind of writes we can rely on our Write to behave how we need it to. Plus it will now let us test the potentially problematic code separately and fix it.

    Let’s write the test where we want to update the entire contents of a file with something that is smaller than the original contents. In tape_test.go:

    Write the test first

    We’ll just create a file, try and write to it using our tape, read it all again and see what’s in the file

    1. === RUN   TestTape_Write
    2. --- FAIL: TestTape_Write (0.00s)
    3.     tape_test.go:23: got 'abc45' want 'abc'

    Write enough code to make it pass

    os.File has a truncate function that will let us effectively empty the file. We should be able to just call this to get what we want.

    Change tape to the following

    1. type tape struct {
    2.     file *os.File
    3. }
    5. func (t *tape) Write(p []byte) (n int, err error) {
    6.     t.file.Truncate(0)
    7.     t.file.Seek(0, 0)
    8.     return t.file.Write(p)
    9. }

    The compiler will fail in a number of places where we are expecting an io.ReadWriteSeeker but we are sending in *os.File. You should be able to fix these problems yourself by now but if you get stuck just check the source code.

    Once you get it refactoring our TestTape_Write test should be passing!

    In RecordWin we have the line json.NewEncoder(f.database).Encode(f.league).

    We don’t need to create a new encoder every time we write, we can initialise one in our constructor and use that instead.

    Store a reference to an Encoder in our type.

    1. type FileSystemPlayerStore struct {
    2.     database *json.Encoder
    3.     league   League
    4. }

    Initialise it in the constructor

    1. func NewFileSystemPlayerStore(file *os.File) *FileSystemPlayerStore {
    2.     file.Seek(0, 0)
    3.     league, _ := NewLeague(file)
    5.     return &FileSystemPlayerStore{
    6.         database: json.NewEncoder(&tape{file}),
    7.         league:   league,
    8.     }
    9. }

    Use it in RecordWin.

    Didn’t we just break some rules there? Testing private things? No interfaces?

    On testing private types

    It’s true that in general you should favour not testing private things as that can sometimes lead to your tests being too tightly coupled to the implementation; which can hinder refactoring in future.

    However, we must not forget that tests should give us confidence.

    We were not confident that our implementation would work if we added any kind of edit or delete functionality. We did not want to leave the code like that, especially if this was being worked on by more than one person who may not be aware of the shortcomings of our initial approach.

    Finally, it’s just one test! If we decide to change the way it works it won’t be a disaster to just delete the test but we have at the very least captured the requirement for future maintainers.

    We started off the code by using io.Reader as that was the easiest path for us to unit test our new PlayerStore. As we developed the code we moved on to io.ReadWriter and then io.ReadWriteSeeker. We then found out there was nothing in the standard library that actually implemented that apart from *os.File. We could’ve taken the decision to write our own or use an open source one but it felt pragmatic just to make temporary files for the tests.

    Finally, we needed Truncate which is also on *os.File. It would’ve been an option to create our own interface capturing these requirements.

    1. type ReadWriteSeekTruncate interface {
    2.     io.ReadWriteSeeker
    3.     Truncate(size int64) error
    4. }

    But what is this really giving us? Bear in mind we are not mocking and it is unrealistic for a file system store to take any type other than an *os.File so we don’t need the polymorphism that interfaces give us.

    Don’t be afraid to chop and change types and experiment like we have here. The great thing about using a statically typed language is the compiler will help you with every change.

    Error handling

    Before we start working on sorting we should make sure we’re happy with our current code and remove any technical debt we may have. It’s an important principle to get to working software as quickly as possible (stay out of the red state) but that doesn’t mean we should ignore error cases!

    If we go back to FileSystemStore.go we have league, _ := NewLeague(f.database) in our constructor.

    NewLeague can return an error if it is unable to parse the league from the io.Reader that we provide.

    It was pragmatic to ignore that at the time as we already had failing tests. If we had tried to tackle it at the same time we would be juggling two things at once.

    Let’s make it so if our constructor is capable of returning an error.

    1. func NewFileSystemPlayerStore(file *os.File) (*FileSystemPlayerStore, error) {
    2.     file.Seek(0, 0)
    3.     league, err := NewLeague(file)
    5.     if err != nil {
    6.         return nil, fmt.Errorf("problem loading player store from file %s, %v", file.Name(), err)
    7.     }
    9.     return &FileSystemPlayerStore{
    10.         database: json.NewEncoder(&tape{file}),
    11.         league:   league,
    12.     }, nil
    13. }

    Remember it is very important to give helpful error messages (just like your tests). People jokingly on the internet say most Go code is

    1. if err != nil {
    2.     return err
    3. }

    That is 100% not idiomatic. Adding contextual information (i.e what you were doing to cause the error) to your error messages makes operating your software far easier.

    If you try and compile you’ll get some errors.

    1. ./main.go:18:35: multiple-value NewFileSystemPlayerStore() in single-value context
    2. ./FileSystemStore_test.go:35:36: multiple-value NewFileSystemPlayerStore() in single-value context
    3. ./FileSystemStore_test.go:57:36: multiple-value NewFileSystemPlayerStore() in single-value context
    4. ./FileSystemStore_test.go:70:36: multiple-value NewFileSystemPlayerStore() in single-value context
    5. ./FileSystemStore_test.go:85:36: multiple-value NewFileSystemPlayerStore() in single-value context
    6. ./server_integration_test.go:12:35: multiple-value NewFileSystemPlayerStore() in single-value context

    In main we’ll want to exit the program, printing the error.

    1. store, err := NewFileSystemPlayerStore(db)
    3. if err != nil {
    4.     log.Fatalf("problem creating file system player store, %v ", err)
    5. }

    In the tests we should assert there is no error. We can make a helper to help with this.

    1. func assertNoError(t *testing.T, err error) {
    2.     t.Helper()
    3.     if err != nil {
    4.         t.Fatalf("didnt expect an error but got one, %v", err)
    5.     }
    6. }

    Work through the other compilation problems using this helper. Finally, you should have a failing test

    1. === RUN   TestRecordingWinsAndRetrievingThem
    2. --- FAIL: TestRecordingWinsAndRetrievingThem (0.00s)
    3.     server_integration_test.go:14: didnt expect an error but got one, problem loading player store from file /var/folders/nj/r_ccbj5d7flds0sf63yy4vb80000gn/T/db841037437, problem parsing league, EOF

    We cannot parse the league because the file is empty. We weren’t getting errors before because we always just ignored them.

    Let’s fix our big integration test by putting some valid JSON in it and then we can write a specific test for this scenario.

    1. func TestRecordingWinsAndRetrievingThem(t *testing.T) {
    2.     database, cleanDatabase := createTempFile(t, `[]`)
    3.     //etc...

    Now all the tests are passing we need to handle the scenario where the file is empty.

    Write the test first

    1. t.Run("works with an empty file", func(t *testing.T) {
    2.     database, cleanDatabase := createTempFile(t, "")
    3.     defer cleanDatabase()
    5.     _, err := NewFileSystemPlayerStore(database)
    7.     assertNoError(t, err)
    8. })

    Try to run the test

    1. === RUN   TestFileSystemStore/works_with_an_empty_file
    2.     --- FAIL: TestFileSystemStore/works_with_an_empty_file (0.00s)
    3.         FileSystemStore_test.go:108: didnt expect an error but got one, problem loading player store from file /var/folders/nj/r_ccbj5d7flds0sf63yy4vb80000gn/T/db019548018, problem parsing league, EOF

    Write enough code to make it pass

    Change our constructor to the following

    1. func NewFileSystemPlayerStore(file *os.File) (*FileSystemPlayerStore, error) {
    3.     file.Seek(0, 0)
    5.     info, err := file.Stat()
    7.     if err != nil {
    8.         return nil, fmt.Errorf("problem getting file info from file %s, %v", file.Name(), err)
    9.     }
    11.     if info.Size() == 0 {
    12.         file.Write([]byte("[]"))
    13.         file.Seek(0, 0)
    14.     }
    16.     league, err := NewLeague(file)
    18.     if err != nil {
    19.         return nil, fmt.Errorf("problem loading player store from file %s, %v", file.Name(), err)
    20.     }
    22.     return &FileSystemPlayerStore{
    23.         database: json.NewEncoder(&tape{file}),
    24.         league:   league,
    25.     }, nil
    26. }

    file.Stat returns stats on our file. This lets us check the size of the file, if it’s empty we Write an empty JSON array and Seek back to the start ready for the rest of the code.

    Refactor

    Our constructor is a bit messy now, we can extract the initialise code into a function

    1. func initialisePlayerDBFile(file *os.File) error {
    2.     file.Seek(0, 0)
    4.     info, err := file.Stat()
    6.     if err != nil {
    7.         return fmt.Errorf("problem getting file info from file %s, %v", file.Name(), err)
    8.     }
    10.     if info.Size()==0 {
    11.         file.Write([]byte("[]"))
    12.         file.Seek(0, 0)
    13.     }
    15.     return nil
    16. }
    1. func NewFileSystemPlayerStore(file *os.File) (*FileSystemPlayerStore, error) {
    3.     err := initialisePlayerDBFile(file)
    5.     if err != nil {
    6.         return nil, fmt.Errorf("problem initialising player db file, %v", err)
    7.     }
    9.     league, err := NewLeague(file)
    11.     if err != nil {
    12.         return nil, fmt.Errorf("problem loading player store from file %s, %v", file.Name(), err)
    13.     }
    15.     return &FileSystemPlayerStore{
    16.         database: json.NewEncoder(&tape{file}),
    17.         league:   league,
    18.     }, nil
    19. }

    Sorting

    Our product owner wants /league to return the players sorted by their scores.

    The main decision to make here is where in the software should this happen. If we were using a “real” database we would use things like ORDER BY so the sorting is super fast so for that reason it feels like implementations of PlayerStore should be responsible.

    Write the test first

    We can update the assertion on our first test in TestFileSystemStore

    1. t.Run("league sorted", func(t *testing.T) {
    2.     database, cleanDatabase := createTempFile(t, `[
    3.         {"Name": "Cleo", "Wins": 10},
    4.         {"Name": "Chris", "Wins": 33}]`)
    5.     defer cleanDatabase()
    7.     store := FileSystemPlayerStore{database}
    9.     got := store.GetLeague()
    11.     want := []Player{
    12.         {"Chris", 33},
    13.         {"Cleo", 10},
    14.     }
    16.     assertLeague(t, got, want)
    18.     // read again
    19.     got = store.GetLeague()
    20.     assertLeague(t, got, want)
    21. })

    The order of the JSON coming in is in the wrong order and our want will check that it is returned to the caller in the correct order.

    Try to run the test

    1. === RUN   TestFileSystemStore/league_from_a_reader,_sorted
    2.     --- FAIL: TestFileSystemStore/league_from_a_reader,_sorted (0.00s)
    3.         FileSystemStore_test.go:46: got [{Cleo 10} {Chris 33}] want [{Chris 33} {Cleo 10}]
    4.         FileSystemStore_test.go:51: got [{Cleo 10} {Chris 33}] want [{Chris 33} {Cleo 10}]
    1. func (f *FileSystemPlayerStore) GetLeague() League {
    2.     sort.Slice(f.league, func(i, j int) bool {
    3.         return f.league[i].Wins > f.league[j].Wins
    4.     })
    5.     return f.league
    6. }

    Easy!

    Wrapping up

    What we’ve covered

    • The Seeker interface and its relation with Reader and .
    • Working with files.
    • Creating an easy to use helper for testing with files that hides all the messy stuff.
    • sort.Slice for sorting slices.
    • Using the compiler to help us make structural changes to the application safely.
    • Most rules in software engineering aren’t really rules, just best practices that work 80% of the time.
    • We discovered a scenario where one of our previous “rules” of not testing internal functions was not helpful for us so we broke the rule.
    • It’s important when breaking rules to understand the trade-off you are making. In our case, we were ok with it because it was just one test and would’ve been very difficult to exercise the scenario otherwise.
    • In order to be able to break the rules you must understand them first. An analogy is with learning guitar. It doesn’t matter how creative you think you are, you must understand and practice the fundamentals.

    Where our software is at

    • We have an HTTP API where you can create players and increment their score.
    • We can return a league of everyone’s scores as JSON.
    • The data is persisted as a JSON file.