Implement custom error handling to Storer interface #590
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,32 @@ | ||
| package main | ||
|
|
||
| import "fmt" | ||
|
|
||
| type ErrorCode int | ||
|
|
||
| type Error struct { | ||
| Code ErrorCode | ||
| Message string | ||
| } | ||
|
|
||
| const ( | ||
| NegativeValue ErrorCode = iota + 1001 | ||
| IDNotFound | ||
| Unknown = 1999 | ||
| ) | ||
|
|
||
| func (e *Error) Error() string { | ||
| return fmt.Sprintf("[%d] %s", e.Code, e.Message) | ||
| } | ||
|
|
||
| // NewError creates a new error with the given enum | ||
|
There was a problem hiding this comment. nitpick: Godoc comments should end with a full-stop, as they appear as sentences in the documentation (https://blog.golang.org/godoc-documenting-go-code). another nitpick: I'd say "given error code" instead of "given enum" - seems a little clearer to me. |
||
| func NewError(code ErrorCode) error { | ||
| switch code { | ||
| case NegativeValue: | ||
| return &Error{code, "Puppy value must be greater than 0"} | ||
| case IDNotFound: | ||
| return &Error{code, "Nonexistent Puppy ID"} | ||
| default: | ||
| return &Error{Unknown, "Unknown error"} | ||
|
There was a problem hiding this comment. This throws away the error code passed in, which may be useful when trying to debug where a strange error is coming from. Perhaps set the string as |
||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| package main | ||
|
|
||
| import ( | ||
| "testing" | ||
|
|
||
| "github.com/stretchr/testify/assert" | ||
| ) | ||
|
|
||
| func TestUnknownError(t *testing.T) { | ||
| a := assert.New(t) | ||
|
|
||
| error := NewError(123) | ||
| a.Equal(error, NewError(Unknown)) | ||
| } | ||
|
|
||
| func TestError(t *testing.T) { | ||
| a := assert.New(t) | ||
|
|
||
| error := Error{1, "message"} | ||
| formattedError := error.Error() | ||
| a.Equal(formattedError, "[1] message") | ||
| } | ||
|
There was a problem hiding this comment. I don't see that you have 100% test coverage with this commit. There are a couple of branches in I will share the magic in a subsequent comment where I have noticed your tests failing... |
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is not clear why you have decided to start at 1001 for the error codes, and also set
Unknownto 1999. What if in future you need >1000 error codes? Since this is an enum, the actual values for the enum are not relevant, unless they map to some external system. In that case you should document that mapping. But if there is one, don't suggest there is one but you're not telling by using a different set of meaningless numbers to the default.I'd suggest starting with "Unknown = iota" and going up from there. That does not give you that artificial limit of the number of error codes when you do eventually get to more than 1000 error codes (which will come up in lab 22).
Perhaps you have chosen to use 1000+ so you get a consistent formatting width when converting the error to a string, but I'd suggest you do formatting at the place where formatting is specified (i.e.
[%04d])