Skip to content

Conversation

@frsyuki
Copy link
Member

@frsyuki frsyuki commented Jan 5, 2016

Makes constructors of MessagePacker and MessageUnpacker protected so
that all users use factory methods of MessagePack interface. This
removes duplication of API and make it easy to understand.

Makes constructors of MessagePacker and MessageUnpacker protected so
that all users use factory methods of MessagePack interface. This
removes duplication of API and make it easy to understand.
@frsyuki
Copy link
Member Author

frsyuki commented Jan 5, 2016

This follows-up #310.
@xerial I think having the single entry point makes it easier to understand/explain/document the API.

@frsyuki
Copy link
Member Author

frsyuki commented Jan 5, 2016

And I think, usually, such entry points are called Factory rather than Config. How do you think about using MessagePack.PackerFactory and MessagePack.UnpackerFactory instead of MessagePack.PackerConfig and MessagePack.UnpackerConfig? This is minor topic relatively.

@frsyuki frsyuki changed the title Make constructors protected to simplify the single entry point Make constructors protected to ensure the single entry point Jan 5, 2016
@xerial
Copy link
Member

xerial commented Jan 6, 2016

@frsyuki
Using protected is ok.

I talked with @komamitsu about class naming and reached to the conclusion that Using xxxFactory both for MessagePacker/Unpacker and Jackson binding is confusing. And I also agree that Config does not usually have a factory method.

How about providing the following method so that user can notice config object is necessary to configure packer and unpacker?
MessagePack.newPacker(PackerConfig, out), newUnpacker(UnpackerConfig, in)

xerial added a commit that referenced this pull request Jan 6, 2016
Make constructors protected to ensure the single entry point
@xerial xerial merged commit 6ecdca1 into develop Jan 6, 2016
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.

3 participants