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

null unmarshalling #10

Closed
jedborovik opened this issue Oct 1, 2015 · 3 comments
Closed

null unmarshalling #10

jedborovik opened this issue Oct 1, 2015 · 3 comments

Comments

@jedborovik
Copy link
Contributor

Unmarshalling and marshalling types in the null package isn't symmetric. For example:

type T struct {
  Name null.String
}

in := []byte(`{"Name":""}`)
var t T
json.Unmarshal(in, &t)
fmt.Printf("%+v\n", t) // => {Name:{NullString:{String: Valid:false}}}
out, _ := json.Marshal(t)
fmt.Println(string(out)) // => {"Name":null}

Why not have in unmarshal to {Name:{NullString:{String: Valid:true}}}? In addition to clearing up the issue shown above it would make the null package incredibly useful for the common issue of how to determine if a RESTful request intends to set a value to the zero value or if it just wasn't included in the request (a PATCH-like request).

Right now the common recommendation is to use pointers for the fields in your structs as described here, but I'd much prefer using the null package for this if I could.

Let me know what you think. I'm happy to create a PR for this if you think this idea has legs.

@guregu
Copy link
Owner

guregu commented Oct 2, 2015

Hi, thanks for the report. Blank string input shouldn't produce a null null.String... this is inconsistent with the rest of the null package, and the README.md documentation. I am not sure why this feature existed, I think it's just a remnant of the old v1 package. Feel free to send a PR or I will take care of it.

I think I will bump this up to v3 just in case people depend on the old behavior.

@guregu
Copy link
Owner

guregu commented Oct 2, 2015

Yep, this is an inconsistency I forgot to take care of when I made v2. I have fixed it and bumped the version up to v3. Thanks!

Fixed in a9db3ac.

@guregu guregu closed this as completed Oct 2, 2015
@jedborovik
Copy link
Contributor Author

You beat me to the PR! Thanks for fixing this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants