Feature: Minimum repeats required

[psmskelton]

There are some use cases for enforcing a minimum number of entries in a repeated field, for example defining a polygon with at least 2 coordinates. I would rather my sender throw an error for failing to provide sufficient information, as opposed to my receiver having to analyse and respond stating it received insufficient data.

Thoughts on this before I look at an implementation?

[tsaubergine]

Sure I think that makes sense, defaulting the min_repeat to 0. It also will reduce the space necessary for sending repeated fields when min_repeat is not 0 (since we only need to encode max-min as the repeated length field).

[tsaubergine]

@philboske please take a look at https://github.com/GobySoft/dccl/pull/96 and let me know what you think.

[psmskelton]

Thanks @tsaubergine; I was somewhere between implementing the differences between commit 120e28b and #96 as I saw the branch the other day. We've had some difficulties in maintaining both an apt repository install to /usr/, and a development build to /usr/local/, which has slowed me down significantly.

The only thing I notice is a slight inconsistency between max_repeat() and min_repeat().

There is an enforcement for specifying dccl_field_options().max_repeat() when there isn't one for dccl_field_options().min_repeat(), even though both have valid (one could argue that max_repeat=1 is useless and thus invalid, though) default values in option_extensions.proto.

dccl::FieldCodecBase::check_repeat_settings() throws on:

• !dccl_field_options().has_max_repeat()
• dccl_field_options().max_repeat() < 1
• dccl_field_options().max_repeat() < dccl_field_options().min_repeat().

If the default value in option_extensions.proto aren't used, why are they there? I would interpret a default value as "If I do not specify this option, the default value will be used". In that case, there is a contradiction between !max_repeat, and max_repeat=1? Unless I'm missing something.

[tsaubergine]

Hi -

I see your point about the inconsistency. My view is that not setting max_repeat on a repeated field is almost certainly an omission in most cases (as you point out the default value of 1 is pretty meaningless as you'd be better off most of the time just using optional). I did remove (https://github.com/GobySoft/dccl/pull/96/commits/a4839a45719d264edef9da27bf4e15156e98dd38) the defaults from option_extensions.proto as you make a valid point there.

min_repeated isn't required as 0 is a sane default, and it needs to not be required to be backwards compatible with messages written prior to this update.