Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create CRUD puppy solution with map and syncMap #574

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,4 @@
# Output of the go coverage tool, specifically when used with LiteIDE
*.out
coverage.txt
*.vscode
Empty file modified 00_hello_world/juliaogris/main.go
100644 → 100755
Empty file.
Empty file modified 00_hello_world/juliaogris/main_test.go
100644 → 100755
Empty file.
34 changes: 0 additions & 34 deletions 03_letters/hsy3418/main.go

This file was deleted.

65 changes: 0 additions & 65 deletions 03_letters/hsy3418/main_test.go

This file was deleted.

22 changes: 22 additions & 0 deletions 06_puppy/hsy3418/main.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package main

import (
"fmt"
"io"
"os"
)

var out io.Writer = os.Stdout

func main() {
p1 := Puppy{Breed: "Poodle", Colour: "White", Value: 1280.5}
p2 := Puppy{Breed: "Poodle", Colour: "Grey", Value: 1340.5}
mapStore := NewMapStore()
syncStore := NewSyncStore()
_ = mapStore.CreatePuppy(p1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you should save the id from CreatePuppy and use that to do the ReadPuppy and not just make the assumption that the puppy ID will be zero. As main() is a user of the store, it should not make assumptions about the internal workings of the store. The store could start IDs at 1, not 0. Since there is no need to assume because CreatePuppy returns the ID, you should just use that ID.

_ = syncStore.CreatePuppy(p2)
puppy, _ := mapStore.ReadPuppy(0)
puppy2, _ := syncStore.ReadPuppy(0)
fmt.Fprintf(out, "Puppy ID %d is %v", puppy.ID, puppy.Value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a \n at the end of this output and the next. This should output a complete line to be user-friendly.

fmt.Fprintf(out, "Puppy ID %d is %v", puppy2.ID, puppy2.Value)
}
18 changes: 18 additions & 0 deletions 06_puppy/hsy3418/main_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package main

import (
"bytes"
"testing"

"github.com/stretchr/testify/assert"
)

func TestMain(t *testing.T) {
assert := assert.New(t)
var buf bytes.Buffer
out = &buf
main()
expected := "Puppy ID 0 is 1280.5Puppy ID 0 is 1340.5"
actual := buf.String()
assert.Equal(expected, actual)
}
50 changes: 50 additions & 0 deletions 06_puppy/hsy3418/mapStore.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package main

import "fmt"

// MapStore is a implementation of storer for the storage of puppies
type MapStore struct {
puppies map[int32]Puppy
nextID int32
}

// NewMapStore creates a new mapStore
func NewMapStore() *MapStore {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again here, this is also a part of the exported package. Please document

return &MapStore{
puppies: map[int32]Puppy{},
}
}

// CreatePuppy adds a nuw puppy to the puppy store and returns the id for the puppy
func (m *MapStore) CreatePuppy(puppy Puppy) int32 {
puppy.ID = m.nextID
m.nextID++
m.puppies[puppy.ID] = puppy
return puppy.ID
}

// ReadPuppy retrieves the puppy for a given id from puppies store
func (m *MapStore) ReadPuppy(id int32) (Puppy, error) {
if _, ok := m.puppies[id]; !ok {
hsy3418 marked this conversation as resolved.
Show resolved Hide resolved
return Puppy{}, fmt.Errorf("puppy with %d ID does not exist", id)
}
return m.puppies[id], nil
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of looking up the map again, when you did it already before, you should save the value from from the first lookup. It means you can't put it all inside the if, but that doesn't matter:

puppy, ok := m.puppies[id]
if !ok {
    ...
}
return puppy, nil

}

//UpdatePuppy updates the puppy for the given id
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Put a space after // like you have for the doc strings above. Do the same for the next comment and also for the comments in syncStore.go

func (m *MapStore) UpdatePuppy(puppy Puppy) error {
if _, ok := m.puppies[puppy.ID]; !ok {
return fmt.Errorf("puppy with %d ID does not exist", puppy.ID)
}
m.puppies[puppy.ID] = puppy
return nil
}

//DeletePuppy delete the puppy for the given id from puppies store
func (m *MapStore) DeletePuppy(id int32) error {
if _, ok := m.puppies[id]; !ok {
return fmt.Errorf("puppy with %d ID does not exist", id)
}
delete(m.puppies, id)
return nil
}
116 changes: 116 additions & 0 deletions 06_puppy/hsy3418/store_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,116 @@
package main

import (
"fmt"
"testing"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/suite"
)

func (suite *storerTestSuite) SetupTest() {
suite.store = suite.makeStore()
suite.nonExistPuppy = Puppy{ID: 123456, Breed: "Poodle", Colour: "White", Value: 1000.5}
suite.toBeCreatedPuppy = Puppy{Breed: "Poodle", Colour: "White", Value: 1000.5}
suite.existsPuppy = Puppy{Breed: "Poodle", Colour: "White", Value: 1280.5}
suite.toBeUpdatedPuppy = Puppy{ID: suite.existsPuppy.ID, Breed: "Poodle", Colour: "White", Value: 2000}
suite.store.CreatePuppy(suite.existsPuppy)
}

type storerTestSuite struct {
suite.Suite
store Storer
makeStore func() Storer
toBeCreatedPuppy Puppy
existsPuppy Puppy
toBeUpdatedPuppy Puppy
nonExistPuppy Puppy
}

func (suite *storerTestSuite) TestCreatePuppy() {
assert := assert.New(suite.T())
testCases := []struct {
title string
input Puppy
}{
{"Create new puppy", suite.toBeCreatedPuppy},
}
for _, tc := range testCases {
tc := tc
suite.T().Run(tc.title, func(t *testing.T) {
id := suite.store.CreatePuppy(tc.input)
assert.True(id > 0)
})
}
}

func (suite *storerTestSuite) TestUpdatePuppy() {
assert := assert.New(suite.T())
testCases := []struct {
title string
inputPuppy Puppy
expectedError error
}{
{"Update puppy successfully", suite.toBeUpdatedPuppy, nil},
{"Update non-existing puppy", suite.nonExistPuppy,
fmt.Errorf("puppy with %d ID does not exist", suite.nonExistPuppy.ID)},
}
for _, tc := range testCases {
tc := tc
suite.T().Run(tc.title, func(t *testing.T) {
err := suite.store.UpdatePuppy(tc.inputPuppy)
assert.Equal(tc.expectedError, err)
})
}
}

func (suite *storerTestSuite) TestReadPuppy() {
assert := assert.New(suite.T())
testCases := []struct {
title string
input int32
expected Puppy
expectedError error
}{
{"Read puppy successfully", suite.existsPuppy.ID, suite.existsPuppy, nil},
{"Read non-existing puppy", suite.nonExistPuppy.ID, Puppy{},
fmt.Errorf("puppy with %d ID does not exist", suite.nonExistPuppy.ID)},
}
for _, tc := range testCases {
tc := tc
suite.T().Run(tc.title, func(t *testing.T) {
readPuppy, err := suite.store.ReadPuppy(tc.input)
assert.Equal(tc.expected, readPuppy)
assert.Equal(tc.expectedError, err)
})
}
}

func (suite *storerTestSuite) TestDeletePuppy() {
assert := assert.New(suite.T())
testCases := []struct {
title string
input int32
expectedError error
}{
{"Delete puppy successfully", suite.existsPuppy.ID, nil},
{"Delete non-existing puppy", suite.nonExistPuppy.ID,
fmt.Errorf("puppy with %d ID does not exist", suite.nonExistPuppy.ID)},
}
for _, tc := range testCases {
tc := tc
suite.T().Run(tc.title, func(t *testing.T) {
err := suite.store.DeletePuppy(tc.input)
assert.Equal(tc.expectedError, err)
})
}
}

func TestStorers(t *testing.T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is more readable if this function is placed at the top of the file before SetupTest. This is where the tests in this file start, so it reads better to start here, and then move onto SetupTest, and then finally the tests themselves.

suite.Run(t, &storerTestSuite{
makeStore: func() Storer { return NewMapStore() },
})
suite.Run(t, &storerTestSuite{
makeStore: func() Storer { return NewSyncStore() },
})
}
60 changes: 60 additions & 0 deletions 06_puppy/hsy3418/syncStore.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
package main

import (
"fmt"
"sync"
)

// SynceStore is a implementation of `Storer` backed by a sync.Map
type SyncStore struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have done really well documenting your interface methods, but you forgot these as well. Since the SyncStore and NewSyncStore are also included as part of the exported package, these should also have documentation around them.

sync.Mutex
sync.Map
nextID int32
}

// NewSyncStore creates a new SyncStore
func NewSyncStore() *SyncStore {
return &SyncStore{}
}

// CreatePuppy adds a nuw puppy to the puppy store and returns the id for the puppy
func (m *SyncStore) CreatePuppy(puppy Puppy) int32 {
m.Lock()
defer m.Unlock()
puppy.ID = m.nextID
m.nextID++
m.Store(puppy.ID, puppy)
return puppy.ID
}

// ReadPuppy retrieves the puppy for a given id from puppies store
func (m *SyncStore) ReadPuppy(id int32) (Puppy, error) {
p, ok := m.Load(id)
if !ok {
return Puppy{}, fmt.Errorf("puppy with %d ID does not exist", id)
}
puppy, _ := p.(Puppy)
return puppy, nil
}

//UpdatePuppy updates the puppy for the given id
func (m *SyncStore) UpdatePuppy(puppy Puppy) error {
m.Lock()
defer m.Unlock()
if _, ok := m.Load(puppy.ID); !ok {
return fmt.Errorf("puppy with %d ID does not exist", puppy.ID)
}
m.Store(puppy.ID, puppy)
return nil
}

//DeletePuppy delete the puppy for the given id from puppies store
func (m *SyncStore) DeletePuppy(id int32) error {
m.Lock()
defer m.Unlock()
if _, ok := m.Load(id); !ok {
return fmt.Errorf("puppy with %d ID does not exist", id)
}
m.Delete(id)
return nil
}
17 changes: 17 additions & 0 deletions 06_puppy/hsy3418/types.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
package main

// Puppy defines the data structure corresponding to a pet
type Puppy struct {
ID int32 `json:"id"`
Value float32 `json:"value"`
Breed string `json:"breed"`
Colour string `json:"colour"`
}

//Storer define standard CRUD operations for puppys
type Storer interface {
CreatePuppy(Puppy) int32
ReadPuppy(ID int32) (Puppy, error)
UpdatePuppy(puppy Puppy) error
DeletePuppy(ID int32) error
}