qitab / cl-protobufs

Common Lisp protocol buffer implementation.
MIT License
84 stars 18 forks source link

Add the field 'container' to field-descriptor #205

Closed copybara-service[bot] closed 4 years ago

copybara-service[bot] commented 4 years ago

Add the field 'container' to field-descriptor

Currently in cl-protobufs we set the 'default' field of the field-descriptor to whatever is said in the lisp schema file. Then if we want to know if it's a) repeated b) vector or list we check the field to determine if the second value is list or vector. Instead we should just make a container field.

This is part of https://github.com/qitab/cl-protobufs/pull/156

bkuehnert commented 4 years ago

unrelated to the PR:

How do you feel about removing the code duplication between field-descriptor and field-data? It's possible to remove field-data completely, and have the is-set, bool-values, and bytes vector included automatically on the message (perhaps they could be slots on the currently empty (?) base-message struct.

Another option, which I like less, is to keep field-data and use it as the sole source of data for creating accessor functions. I like this less since we will still have data duplication between the field-data struct and the field-descriptor class.

Slids commented 4 years ago

unrelated to the PR:

How do you feel about removing the code duplication between field-descriptor and field-data? It's possible to remove field-data completely, and have the is-set, bool-values, and bytes vector included automatically on the message (perhaps they could be slots on the currently empty (?) base-message struct.

Another option, which I like less, is to keep field-data and use it as the sole source of data for creating accessor functions. I like this less since we will still have data duplication between the field-data struct and the field-descriptor class.

SGTM