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

Add Serialization for protobuf enum values #11

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add Serialization for protobuf enum values #11

wants to merge 1 commit into from

Conversation

tomjack
Copy link
Contributor

@tomjack tomjack commented Aug 29, 2013

This adds another Serialization impl, for the actual enum values (not the enum value descriptors, like ProtobufEnumSerialization).

I used reflection to get at the enum classes "valueOf". Since I only get the method once, I hope it does not add terrible overhead.

By the way, ProtobufEnumSerialization seems incomplete — deserialize returns null. Do we also want serialization for the enum value descriptors? I guess the motivation was that getField for an enum field returns a value descriptor instead of the actual value?

@bryanduxbury
Copy link
Contributor

You are correct about why there's no enum value serialization - getField for an enum returns a descriptor and not a type. I'm not averse to adding a serializer for enum values... but how would one go about extracting such a value? The only way I can think of right now is through a custom function.

In any case, I would like to see a unit test that covers this new code before merging.

Also, it looks like the existing enum deserializer just plain wasn't implemented all the way. That needs to be remedied.

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.

2 participants