ci/score: rename DumpAsJSON()

meling commented 9 years ago

func (s *Score) DumpAsJSON(t *testing.T) ==> func (s *Score) MarshalJSON() ([]byte, error)

This means that Score would be implementing the Marshaler interface, which states: Marshaler is the interface implemented by objects that can marshal themselves into valid JSON.

Thus, in tests instead of sc.DumpAsJSON(t) we would need to do

json, _ := sc.MarshalJSON()
t.Log(json)

Or,

json, _ := sc.MarshalJSON()
fmt.Println(json)

The drawback is that we go from one to two lines and expose error handling, and need to cast from []byte to string.

Another solution is to not implement the Marshaler interface and return a string and no error, as below:

func (s *Score) DumpAsJSON(t *testing.T) ==> func (s *Score) JSON() string

Thus, in tests instead of sc.DumpAsJSON(t) we would need to do t.Log(sc.JSON()) or fmt.Println(sc.JSON()).

Not sure when the json.Marshal() function would return an error, but we could either ignore it, or create an empty Score object and return JSON for that, or we could include the error message in the JSON string. For example: {"Error":"{{.err}}"}, where the err is placed in the JSON. Not sure if that makes sense.

I think the last version is the most appealing. Comments welcome.

tormoder commented 9 years ago

This means that Score would be implementing the Marshaler interface, which states: Marshaler is the interface implemented by objects that can marshal themselves into valid JSON.

Why would we implement MarshalJSON when we can just call b, err := json.Marshal(score)?

If we implement MarshalJSON() we have to implement the marshaling manually ourselves. json.Marshal checks if a type implements Marshaler, and if so calls MarshalJSON.

Implementing it as:

func (s *Score) MarshalJSON() ([]byte, error) {
    return json.Marshal(s)
}

would not work.

The following will cause an stack overflow due to recursion. json.Marshal calls s.MarshalJSON calls json.Marshal and so on.

package main

import (
    "encoding/json"
    "fmt"
)

type Score struct {
    Name string
}

func (s *Score) MarshalJSON() ([]byte, error) {
    return json.Marshal(s)
}

func main() {
    s := &Score{Name: "Foo"}
    j, err := s.MarshalJSON()
    fmt.Println("JSON:", j)
    fmt.Println("Err:", err)
}

tormoder commented 9 years ago

I agree that the methods can be more general, but it would in addition be nice to have some function/method-wrappers for printing output in tests (that also handles errors). They don't necessarily need to be in the score package, but it would be nice to keep them in one place.

hfurubotten / autograder

ci/score: rename DumpAsJSON() #12