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

Make all fields optional and set sensibe default values #53

Open
machinekoder opened this issue Jan 18, 2016 · 6 comments
Open

Make all fields optional and set sensibe default values #53

machinekoder opened this issue Jan 18, 2016 · 6 comments

Comments

@machinekoder
Copy link
Member

It is good design practice to make all fields optional. It is not only recommended with Protocol Buffers but also Apache Thrift which has similar IDL language:

Any new fields that you add should be optional. This means that any messages serialized by code
using your "old" message format can be parsed by your new generated code, as they won’t be
missing any required elements. You should set up sensible default values for these elements so that
new code can properly interact with messages generated by old code. Similarly, messages created
by your new code can be parsed by your old code

See: http://diwakergupta.github.io/thrift-missing-guide/thrift.pdf

With Protobuf 3 there only will be optional fields as a result of feedback to the problem.

@machinekoder machinekoder changed the title Make all fields optional Make all fields optional and set sensibe default values Jan 18, 2016
@machinekoder
Copy link
Member Author

Another good tip from the thrift guide:

Non-required fields can be removed, as long as the tag number is not used again in your
updated message type (it may be better to rename the field instead, perhaps adding the prefix
"OBSOLETE_", so that future users of your .thrift can’t accidentally reuse the number).

@machinekoder
Copy link
Member Author

Protobuf3 removes the option to set default values instead type specific values are used.

@dkhughes
Copy link

dkhughes commented Jul 6, 2017

@machinekoder Is a move to proto3 in the works? The new language support (c#, go, etc.) is only available in proto3+. Plus, they promise not to change the API as fluidly as before. nanopb has supported proto3 since 0.3.7, but I don't think Jessie is supporting it. Would probably need to make a package like czmq. Pretty sure stretch is already supporting proto3 with the default packages.

@machinekoder
Copy link
Member Author

@dkhughes You can already compile Machinekit with Protobuf 3.x.

@machinekoder
Copy link
Member Author

Regarding default values, I have no idea what Protobuf 3.x does with proto2 files when it comes to default values.

@dkhughes
Copy link

dkhughes commented Jul 6, 2017

I meant proto3 syntax in the proto files. Protoc will not generate code for proto2 files in some of the newer languages.

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