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

Added json marshal/unmarshal methods and structs #5

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
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
34 changes: 34 additions & 0 deletions range.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
package trn

import (
"encoding/json"
"fmt"
"strconv"
"strings"
Expand Down Expand Up @@ -51,6 +52,11 @@ type Range struct {
dur time.Duration
}

type marshalTime struct {
StartTime time.Time `json:"StartTIme"`
Copy link
Member

Choose a reason for hiding this comment

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

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

EndTime time.Time `json:"EndTime"`
}

// String implements fmt.Stringer to print and log Range properly
func (r Range) String() string { return r.UTC().Format(defaultRangeFmt) }

Expand Down Expand Up @@ -182,6 +188,34 @@ func (r Range) Flip(ranges []Range) []Range {
return r.flipValidRanges(rngs)
}

func (r Range) MarshalJSON() ([]byte, error) {
data := marshalTime{
StartTime: r.st,
EndTime: r.End(),
}

jsonData, err := json.Marshal(data)
if err != nil {
fmt.Println("Error marshaling JSON:", err)
Copy link

Choose a reason for hiding this comment

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

why do force print needed here. Could you provide more context please where do you need use stdout f.e. in standard libraries

Copy link
Member

Choose a reason for hiding this comment

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

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)

return nil, err
}

return jsonData, nil
}

func (r *Range) UnmarshalJSON(jsonData []byte) error {
var data marshalTime

if err := json.Unmarshal(jsonData, &data); err != nil {
return err
Copy link
Member

Choose a reason for hiding this comment

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

This error could be wrapped to add additional context to the error itself, for better traceability

Copy link
Member

Choose a reason for hiding this comment

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

I think this method lacks some tests, it would be better if it would be covered.

}

r.st = data.StartTime
r.dur = data.EndTime.Sub(data.StartTime)

return nil
}

func (r Range) flipValidRanges(ranges []Range) []Range {
var res []Range

Expand Down
49 changes: 49 additions & 0 deletions range_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package trn

import (
"encoding/json"
"testing"
"time"

Expand Down Expand Up @@ -515,3 +516,51 @@ func TestRange_GoString(t *testing.T) {
func TestError_Error(t *testing.T) {
assert.Equal(t, "blah", Error("blah").Error())
}
func TestRangeMarshalJSON(t *testing.T) {
// Create a sample Range instance
startTime := time.Date(2023, time.September, 6, 12, 0, 0, 0, time.UTC)
duration := time.Hour * 2
r := Range{st: startTime, dur: duration}

// Marshal the Range to JSON
jsonData, err := r.MarshalJSON()
if err != nil {
Copy link
Member

Choose a reason for hiding this comment

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

I would prefer to use the assert library for such assertions, as it is used in the other tests

t.Fatalf("Error marshaling Range to JSON: %v", err)
}

// Unmarshal the JSON data back into a Range
var unmarshaledRange Range
err = json.Unmarshal(jsonData, &unmarshaledRange)
if err != nil {
t.Fatalf("Error unmarshaling JSON to Range: %v", err)
}

// Check if the unmarshaled Range has the same start and end time components
if !unmarshaledRange.st.Equal(startTime) || unmarshaledRange.dur != duration {
t.Errorf("Expected: %v, Got: %v", r, unmarshaledRange)
}
}

func TestRangeUnmarshalJSON(t *testing.T) {
// Define a JSON string representing a Range
jsonData := []byte(`{"StartTime":"2023-09-06T12:00:00Z","EndTime":"2023-09-06T14:00:00Z"}`)

// Create an empty Range
var r Range

// Unmarshal the JSON data into the Range
err := r.UnmarshalJSON(jsonData)
if err != nil {
t.Fatalf("Error unmarshaling JSON to Range: %v", err)
}

// Define the expected Range
expectedStartTime := time.Date(2023, time.September, 6, 12, 0, 0, 0, time.UTC)
expectedEndTime := time.Date(2023, time.September, 6, 14, 0, 0, 0, time.UTC)
expectedRange := Range{st: expectedStartTime, dur: expectedEndTime.Sub(expectedStartTime)}

// Check if the unmarshaled Range has the same start and end time components
if !r.st.Equal(expectedStartTime) || r.dur != expectedRange.dur {
t.Errorf("Expected: %v, Got: %v", expectedRange, r)
}
}