Lab6 #469
Lab6 #469
Conversation
CameronSchafer
requested review from
anzboi,
anzdaddy,
camscale,
joshcarp,
juliaogris,
linuxmonk,
pentaphobe and
rokane
as code owners
CameronSchafer
force-pushed
the
lab6
branch
2 times, most recently
from
308fea3 to
b52ad8e
Compare
Codecov Report
@@ Coverage Diff @@
## master #469 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 46 47 +1
Lines 776 833 +57
=====================================
+ Hits 776 833 +57
Continue to review full report at Codecov.
|
| CreatePuppy(*Puppy) int | ||
| ReadPuppy(int) *Puppy | ||
| UpdatePuppy(*Puppy) | ||
| DeletePuppy(int) |
There was a problem hiding this comment.
These interface functions do not return error at all. Wonder how does function caller know something is wrong, e.g. id not found?
| CreatePuppy(*Puppy) int | ||
| ReadPuppy(int) *Puppy | ||
| UpdatePuppy(*Puppy) | ||
| DeletePuppy(int) |
| store sync.Map | ||
| } | ||
|
|
||
| type Storer interface { |
There was a problem hiding this comment.
move to storer.go or types.go file.
| store map[int]Puppy | ||
| } | ||
|
|
||
| type SyncStore struct { |
There was a problem hiding this comment.
move to syncstore.go file along with its implementation
| value string | ||
| } | ||
|
|
||
| type MapStore struct { |
There was a problem hiding this comment.
move to mapstore.go file along with its implementation
| } | ||
|
|
||
| func (m *MapStore) CreatePuppy(p *Puppy) int { | ||
| m.store[p.pid] = *p |
There was a problem hiding this comment.
IMO Create Puppy should provide the ID (the client/user of your package typically would not know what ID's are available). However you can make a case for the client providing the ID if you wish. In that case you need to check that the ID isn't already in use and return an error otherwise please.
| } | ||
|
|
||
| // using the sync map store + methods | ||
| func usingSyncMap(pups []Puppy) { |
There was a problem hiding this comment.
usingSyncMap and usingNormMap look nearly identical.
pull store initialisation out and pass map|sync store via interface argument along with puppy slice to a
managePuppies(s Stoerer, pups []Puppy) function or similar.
Fixes #470
Review of colleague's PR #265
Changes proposed in this PR: