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

Addtional examples of bindnode usage. #209

Closed
wants to merge 1 commit into from

Conversation

warpfork
Copy link
Collaborator

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.

Including exercise of:

- stringjoin representations of structs

- stringprefix representations of unions

- schema field names that do not directly equal go struct field names

- deserialization!
@warpfork warpfork requested a review from mvdan July 23, 2021 00:26
))
schemaType := ts.TypeByName("FooBarBaz")

type FooBarBaz struct {
Copy link
Member

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?

Copy link
Contributor

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.

Copy link
Contributor

@mvdan mvdan left a 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 {
Copy link
Contributor

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)
Copy link
Contributor

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())
Copy link
Contributor

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)
Copy link
Contributor

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

node/bindnode/example_test.go Show resolved Hide resolved
dagjson.Encode(nodeRepr, &buf)

// Output how this was serialized, for the example.
fmt.Fprintf(os.Stdout, "json: %s\n", buf.Bytes())
Copy link
Contributor

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)
Copy link
Contributor

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.

@mvdan
Copy link
Contributor

mvdan commented Aug 12, 2021

I've incorporated a modified version of the union example for #223.

schema field names that do not directly equal go struct field names

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.

stringjoin representations of structs

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 :)

@mvdan
Copy link
Contributor

mvdan commented Sep 10, 2021

I think we can close this; I already incorporated most of these in a different form in another PR of mine.

@mvdan mvdan closed this Sep 10, 2021
@mvdan mvdan deleted the more-bindnode-examples branch January 10, 2022 15:15
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

Successfully merging this pull request may close these issues.

None yet

4 participants