Added json marshal/unmarshal methods and structs #5
Conversation
|
@Semior001 can you have a look at it. |
range.go
Outdated
| @@ -51,6 +52,11 @@ type Range struct { | |||
| dur time.Duration | |||
| } | |||
|
|
|||
| type MarshalTime struct { | |||
There was a problem hiding this comment.
I actually think that the type could be private, as we want to hide the logic of marshalling and unmarshalling behind the MarshalJSON and UnmarshalJSON methods of Range type
range.go
Outdated
| fmt.Println("JSON Data:", string(jsonData)) | ||
| } | ||
|
|
||
| func (r *Range) UnmarshalStartEndTime(jsonData []byte) error { |
There was a problem hiding this comment.
Method names should be UnmarshalJSON and MarshalJSON
| var data MarshalTime | ||
|
|
||
| if err := json.Unmarshal(jsonData, &data); err != nil { | ||
| return err |
There was a problem hiding this comment.
This error could be wrapped to add additional context to the error itself, for better traceability
There was a problem hiding this comment.
I think this method lacks some tests, it would be better if it would be covered.
|
Thank you for your contribution! |
range.go
Outdated
| @@ -182,6 +188,34 @@ func (r Range) Flip(ranges []Range) []Range { | |||
| return r.flipValidRanges(rngs) | |||
| } | |||
|
|
|||
| func (r Range) MarshalStartEndTime() { | |||
There was a problem hiding this comment.
could you please provide more context with use cases where this is required.
|
|
||
| jsonData, err := json.Marshal(data) | ||
| if err != nil { | ||
| fmt.Println("Error marshaling JSON:", err) |
There was a problem hiding this comment.
why do force print needed here. Could you provide more context please where do you need use stdout f.e. in standard libraries
There was a problem hiding this comment.
I agree with that point. An average consumer of the library don't want an arbitrary error to appear in stdout, but rather to print the error, that has been returned by the function on his own. Also, I would recommend you to wrap the error with the message, like "unmarshal json data", e.g.:
return nil, fmt.Errorf("unmarshal start and date: %w", err)
|
|
||
| jsonData, err := json.Marshal(data) | ||
| if err != nil { | ||
| fmt.Println("Error marshaling JSON:", err) |
There was a problem hiding this comment.
I agree with that point. An average consumer of the library don't want an arbitrary error to appear in stdout, but rather to print the error, that has been returned by the function on his own. Also, I would recommend you to wrap the error with the message, like "unmarshal json data", e.g.:
return nil, fmt.Errorf("unmarshal start and date: %w", err)|
|
||
| // Marshal the Range to JSON | ||
| jsonData, err := r.MarshalJSON() | ||
| if err != nil { |
There was a problem hiding this comment.
I would prefer to use the assert library for such assertions, as it is used in the other tests
| @@ -51,6 +52,11 @@ type Range struct { | |||
| dur time.Duration | |||
| } | |||
|
|
|||
| type marshalTime struct { | |||
| StartTime time.Time `json:"StartTIme"` | |||
There was a problem hiding this comment.
I would prefer to use snake case keys for json values, e.g. "start_time" and "end_time". Also, I'm not sure that the tests will pass, as this key has "I" letter in the upper case, when in the test it is in the lower case
Closes #4