-
Notifications
You must be signed in to change notification settings - Fork 50
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
Addtional examples of bindnode usage. #209
Conversation
Including exercise of: - stringjoin representations of structs - stringprefix representations of unions - schema field names that do not directly equal go struct field names - deserialization!
)) | ||
schemaType := ts.TypeByName("FooBarBaz") | ||
|
||
type FooBarBaz struct { |
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.
How is this mapping working? Does it just use field order to figure out what to assign where? So the schema could have any arbitrary name but the values get assigned to whatever order is there?
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.
Yes, when mapping a provided IPLD schema with a provided Go type, it just cares about the "shape" matching, not any names. For a struct, that means that the fields should have matching types in the same order.
Doing the mapping via names or struct field tags would also be an option, though it would require more code and features, so I initially just went for the simplest approach from my point of view.
I haven't documented this, mind you, because I'm not 100% certain that this default behavior is right. I think most users would expect case-insensitive-matching by field name, similar to most other Go libraries. I'm not bothered by this example being public, but the semantics in this case might change.
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.
I wonder if we should join these examples into one, and call it "with custom representation". After all, the examples should show the different features of the bindnode package, not the different features of IPLD schemas :)
)) | ||
schemaType := ts.TypeByName("FooBarBaz") | ||
|
||
type FooBarBaz struct { |
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.
Yes, when mapping a provided IPLD schema with a provided Go type, it just cares about the "shape" matching, not any names. For a struct, that means that the fields should have matching types in the same order.
Doing the mapping via names or struct field tags would also be an option, though it would require more code and features, so I initially just went for the simplest approach from my point of view.
I haven't documented this, mind you, because I'm not 100% certain that this default behavior is right. I think most users would expect case-insensitive-matching by field name, similar to most other Go libraries. I'm not bothered by this example being public, but the semantics in this case might change.
// Take the representation of the node, and serialize it. | ||
nodeRepr := node.Representation() | ||
var buf bytes.Buffer | ||
dagjson.Encode(nodeRepr, &buf) |
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.
check error?
dagjson.Encode(nodeRepr, &buf) | ||
|
||
// Output how this was serialized, for the example. | ||
fmt.Fprintf(os.Stdout, "json: %s\n", buf.Bytes()) |
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.
nit: for the sake of keeping the example simpler, you could just dagjson.Encode straight to os.Stdout, printing json:
in the line above or in the same line via Printf
fmt.Fprintf(os.Stdout, "json: %s\n", buf.Bytes()) | ||
|
||
// Now unmarshal that again and print that too, to show it working both ways. | ||
np := bindnode.Prototype(&FooBarBaz{}, schemaType) |
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.
nit: you could just do node.Prototype()
here, unless you want to showcase bindnode.Prototype
too
dagjson.Encode(nodeRepr, &buf) | ||
|
||
// Output how this was serialized, for the example. | ||
fmt.Fprintf(os.Stdout, "json: %s\n", buf.Bytes()) |
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.
same comments as in the other example: error and buffer :)
fmt.Fprintf(os.Stdout, "json: %s\n", buf.Bytes()) | ||
|
||
// Now unmarshal that again and print that too, to show it working both ways. | ||
np := bindnode.Prototype((*FooOrBar)(nil), schemaType) |
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.
nit: in one example you use &T{}
, and in the other you use (*T)(nil)
. the latter is slightly better, as it avoids an alloc. but you should also be consistent.
I've incorporated a modified version of the union example for #223.
I actually think we should delay exposing an example for this, because it's quite likely I'll change how this works - think Go field tags. I also want to implement checks that ensure a Go type and a schema type are valid when supplied together. This does not exist right now; bindnode blindly trusts the user. So it's a bit dangerous to encourage the users to do funky things with mismatching field names right now.
There's nothing particularly special about IPLD schema structs, and the other examples already cover structs and custom representation strategies, so I'm inclined to say we don't want this example. We want to keep the number of doc examples relatively low, I'd say between 3-6 ideally. With my unions PR, and with a future "override field names" example, we're already at 5. I don't want to end up with two or three times as many examples, as then the user would just be overloaded with information :) |
I think we can close this; I already incorporated most of these in a different form in another PR of mine. |
I added a few more examples of bindnode usage while poking around to make sure I knew for myself what it can do :)
Including exercise of:
stringjoin representations of structs
stringprefix representations of unions
schema field names that do not directly equal go struct field names
deserialization!
I'm not sure if they're ideal documentation; they're just what I ended up plonking out for myself to see.