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

Lab 6 - CRUD puppy with interface #533

Closed
wants to merge 3 commits into from
Closed

Conversation

undrewb
Copy link
Collaborator

@undrewb undrewb commented Jul 3, 2019

Fixes #532

Review of colleague's PR #424

Changes proposed in this PR:

Implement a Puppy struct containing ID, Breed, Colour, Value.
Create Storer interface with CRUD methods for Puppy
Write a MapStore implementation of Storer backed by a map
Write a SyncStore implementation of Storer backed by a sync.Map
Keep all implementation files in the same folder and in package main
Test against the Storer interface and run in suite with both implementations

Create puupy struct
Implement Init to initialize a puppy and assign a random id
Implement String method to print a puppy struct
At this point there is no check for collision ids
Implement a map backed storer only at this time

Implement ReadStore

Error checking hasnt been implemented.
At this stage nil will be returned if the item cant be found.
This will be addressed in a later lab

Implement DeletePuppy
Fix boolean logic in ReadPuppy

Implement UpdatePuppy
Implement clone method on puppy
Resolve problem with usage of pointers and references

Separate MapStore into its own file
Create a basic main_test

Integrate linter feedback

Rename ID to id where used as parameter

Change mapstore test to use a test suite using the Storer interface
Begin implementing a syncstore version of the Storer interface

Implement all methids for syncstore
Implement all cases for store_tests
Ensure 100% test coverqge

Fix linter errors

Capitals and newlines in error messages
Not assign table tests when ranging across them
Misalignment of structs

Verified lint and code cov passed

Separate types into their own file

Resolve linting issue in types.go
@codecov
Copy link

codecov bot commented Jul 3, 2019

Codecov Report

Merging #533 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #533   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          79     83    +4     
  Lines        1345   1404   +59     
=====================================
+ Hits         1345   1404   +59
Impacted Files Coverage Δ
06_puppy/undrewb/MapStore.go 100% <100%> (ø)
06_puppy/undrewb/SyncStore.go 100% <100%> (ø)
06_puppy/undrewb/types.go 100% <100%> (ø)
06_puppy/undrewb/main.go 100% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 071f548...6024201. Read the comment docs.

Copy link
Collaborator

@willshen8 willshen8 left a comment

Choose a reason for hiding this comment

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

It's mostly pretty good.

  1. Uncommit gitignore file.
  2. Fix up your test cases.

.gitignore Outdated
@@ -11,3 +11,4 @@
# Output of the go coverage tool, specifically when used with LiteIDE
*.out
coverage.txt
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oops, this file should have not been committed.

puppy.Value = p.Value
return puppy
}

Copy link
Collaborator

Choose a reason for hiding this comment

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

I like the idea of immutability.

06_puppy/undrewb/MapStore.go Outdated Show resolved Hide resolved
puppy: corgi,
wantErr: true},
}
for _, tt := range tests {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This is just a comment. I have written my test cases as unit test cases, as each one test a specific condition. Rather than bundle all the create operations into one. Not saying it's right or wrong. But I think test cases should be atomic. And it will eliminate the need for struct, and making test files shorter.

06_puppy/undrewb/store_test.go Outdated Show resolved Hide resolved
06_puppy/undrewb/store_test.go Show resolved Hide resolved
06_puppy/undrewb/store_test.go Show resolved Hide resolved
06_puppy/undrewb/store_test.go Show resolved Hide resolved
Bucknell, Andrew added 2 commits August 22, 2019 23:19
Change format of test case data
Change parameter ordering of test cases
Implement ID assignment in storers
@juliaogris
Copy link
Contributor

The go-course is now closed. Thank you very much for participating.

@juliaogris juliaogris closed this Feb 8, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lab 6 - CRUD puppy with interface
4 participants