Skip to content

Mechanism to generate Extension functions #35

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

Closed
dbudworth opened this issue Nov 29, 2014 · 6 comments
Closed

Mechanism to generate Extension functions #35

dbudworth opened this issue Nov 29, 2014 · 6 comments

Comments

@dbudworth
Copy link

When one simply wants to create a set of custom types to be used as a field of type interface{}, you really just want to register a type number with a normal struct.

Currently, you must add 4 boiler plate functions to satisfy the Extension interface.

Is there a reason for making the extension interface not use the same function names as (Un)Marshaller and add the additional ExtensionType function?

Example code I have copy/pasted (and renamed struct) in my code:

func (i *Identity) ExtensionType() int8 {
    return IdentityT
}
func (i *Identity) Len() int {
    return i.Msgsize()
}
func (i *Identity) MarshalBinaryTo(b []byte) error {
    _, err := i.MarshalMsg(b)
    return err
}
func (i *Identity) UnmarshalBinary(b []byte) error {
    _, err := i.UnmarshalMsg(b)
    return err
}
@dbudworth dbudworth changed the title Mechanism to generate Extension type functions / Rename Extension functions to match generated Mechanism to generate Extension functions Nov 29, 2014
@philhofer
Copy link
Member

Msgsize() does not return the exact size of the object; it over-estimates by a bit. So using it for Len() will return the wrong value.

You could always define a type like

type Any interface {
    msgp.Marshaler
    msgp.Unmarshaler
}

type Abstract struct {
    Type int8
    Value Any
}

type MyStruct struct {
    Thing1 Abstract
    Thing2 Abstract
    /* ... and so forth */
}

But I'm not quite sure I understand your use-case.

@dbudworth
Copy link
Author

Use case is to have an arbitrary type as a member of a generic message.
for example:

type Msg struct {
    Send time.Time
    Body interface{}
}

type Identity struct { Address string }
type Discovered struct { Address string }

then, do something like:

var msg Msg
msg.DecodeMsg(r)
switch t := msg.Body.(type){
    case Identity:
    case Discovered:
}

Could accomplish basically the same thing if Msg looked like

type Msg struct {
    Send time.Time
    Type int8
    Body []byte
}

But was looking to do something a bit more automagical.

@dbudworth
Copy link
Author

I think this still may not be clear.
In the most basic sense, I'm trying to send various structs over the wire and have them turned back in to structs on the other side

gob does this by encoding the type description when it first encounters a type

msgp does this via extension types

I was looking to lower the amount of hand written code, per type, to make this happen.

@philhofer
Copy link
Member

Ok. I've given this some thought.

I would discourage you from using extensions to achieve polymorphism; that is not the intended purpose of extensions in the protocol spec. They are intended for opaque application-specific binary that is not (necessarily) self-describing. (time.Time is a good example.) The extension facilities in this package are not designed to provide the user with polymorphism

What you want to do is encode your polymorphic objects as [type, data] (in other words, as an array of length 2), and use the first number in the array (probably just an int8) to choose a constructor to use on the second object. Right now, you would have to do this manually, although it would amount to less than 100 lines of code per "set" of objects. Fortunately, though, it is probably less code than doing reflection on the interface{} afterwords (or at least about the same amount), and it is more performant, because you never actually need to do reflection. Also, that representation will work just fine in any other MessagePack library, so you won't run into protocol portability problems. (Using extensions across packages requires extra bookkeeping.)

I'm going to see if I can find a nice way to generate code that will do this for you, but for now I would recommend doing it manually using the facilities provided in the package.

I would also encourage you to take a look at the struct decoding rules in the wiki. Since the generated methods decode an intersection of the encoded data and the struct, you can set up your structs with nullable fields and then check post-decoding what values existed in the message. That is the preferred method of "polymorphism."

@dbudworth
Copy link
Author

Yeah, I was thinking about this a bit more too. And I ended up going with your initial (and this) suggestion of having a type + data.

I was able to minimize the work needed by just having a DecodeBody that does a switch and constructs a struct to return (I made a Command interface that all structs comply with)
So I basically do:
msg.DecodeBody().Execute(context)
(removed the error handling for clarity)

works fine

Not sure if there's anything you can do easily generation wise other than maybe have a type map
map[int]InterfaceSpecifiedInCommentForGenerator

// generate: msgp -auto Foo

which would create a FooRegistry where you can FooRegistry.Add(1,MyType{})

you would have to reflect on the encode function to know the type num to use
or make an Autoable interface (like Extension) but with only one GetType function

Still, feels somewhat counter to your goal on this project which I take to be about speed and relative simplicity for the user.

I'd probably leave it out all together and make knuckleheads like me do nullable fields or 2 elem array style, but hand rolled as I think no generalized version would be worth the added complexity to your API

Thank you for your help, and an awesome library.

Feel free to close this, I'm leaving open for now so you see the response.

@philhofer
Copy link
Member

Cool. Let me know if anything else comes up.

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