Re: Using enums to avoid default true in "settings dictionaries" (#466, #467, #471)

On 2016-02-17 10:36, Adam Bergkvist wrote:
> Hi
>
> We currently have three PRs (#466 [1], #467 [2], #471 [3]) that attempt
> to solve the "avoid having true as default value" problem [4] for
> dictionaries by replacing them with enums. This mail tries to summarize
> the situation and present some options going forward. The goal is to be
> somewhat consistent so we can use the outcome of this discusion to make
> future decisions.
>
> #467 [2] and #471 [3] present similar cases where we have an attribute
> with a "boolean-like" name and the enum would have two values represent
> "on" and "off".
>
> - boolean voiceActivityDetection = true;
> + RTCVoiceActivityOption voiceActivityDetection = "enabled";
> (other value is "disabled)
>
> - boolean ordered = true;
> + (boolean or RTCDataChannelOrdered) ordered = "ordered";
> (other value is "unordered")
>
> About the above changes:
> * It solves the "true as default value" problem.
>
> * Verbosity without extra clarity. We don't get the clarity you get when
> replacing a boolean function argument with an enum (e.g. f(true) vs
> f("enableFeature")) here since we're dealing with dictionaries that
> always have the member name in front of the value (e.g. ordered: "ordered").
>
> * In the case of ordered, the RTCDataChannel has a property that
> reflects the setting. Developers might expect the expression dc.ordered
> to work as is, but it evaluates to true for both "ordered" and "unordered".
>
> An alternative approach could be to keep the boolean types but rephrase
> the names to indicate that they are turning off a feature that is on by
> default. For example
>
> boolean unordered = false;
> boolean disableVoiceActivityDetection = false;
>
> Our options: Keep the current (1), switch to enums (2) or keep boolean
> but change the names (3).
>
> #471 [3] is a bit different since it aims to replace two boolean
> properties with a single enum with four values.
>
> - boolean send = true;
> - boolean receive = true;
> + RTCRtpTransceiverDirection direction = "sendrecv";
> (other values: "sendonly", "recvonly" and "inactive")
>
> Alternative approaches:
>
> (C) Negate the names and keep booleans.
>
> boolean deactivateSender = false;
> boolean deactivateReceiver = false;
>
> (D) Use two separate values with on/off-like enum values.
>
> RTCRtpActivity sender = "active";
> RTCRtpActivity receiver = "active";
> (other value "inactive")
>
> Our options: Let's call what's in the spec currently approach (A), and
> the enum proposal in PR #471 (B). The rest is listed above.

For #467 and #471 my preference is to keep the booleans and change the 
names (3). I don't think we gain much by inventing a new boolean in this 
particular case That way the property on RTCDataChannel that reflects 
the "order mode" could be a boolean.

For #466 I think the current approach in the PR with four enum values is 
fine. I don't necessarily think we need to stick to the short names used 
in SDP. "sendrecv" could become "send-receive".

/Adam

Received on Wednesday, 17 February 2016 09:50:49 UTC