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

Harald is right :).

Despite my previous comments in the PRs, the more I think about this, the
more I feel like we need to avoid contorting the API and making it worse
for developers.  From the perspective have the JS application using these,
I feel that the following are clearly superior to read and write:

{ordered: false}
{send: false, receive: false}

I'm fine with {voiceActivityDetection: "disabled"} for readability and
writability, but I'm not so convinced it's worth going to the trouble of
breaking anyone currently using {voiceActivityDetection: false}.


The PR for RTCDataChannelOrdered succeeds in allowing it to be a bool,
which is nice.  I think that if we go down this route with enums we should
do the same for voiceActivityDetection and send/recv, and leave the names
send/recv the same.  Thus send/recv would just change type to
"(boolean or RTCRtpActivity)".



But I'm still left wondering:  is all of this really worth it?  The
original thread about this topic said it was trying to avoid having
createOffer({voiceActivityDetection: undefined}) turn on VAD.  But I fail
to see how any of the PRs changes that kind of behavior.  Wouldn't
createOffer({voiceActivityDetection: undefined}) turn into
createOffer({voiceActivityDetection: "enabled"})?

I'm left wondering: if it's just "bad WebIDL form" to have true-ish
defaults, why don't we just leave the defaults "undefined" and make the
meaning of "undefined" on dictionaries, when passed in their respective
methods, be "ordered data channel", "sender is active", "receiver is
active", and "VAD is enabled"?  Wouldn't that be a lot more simple?







On Wed, Feb 17, 2016 at 8:05 AM, Harald Alvestrand <harald@alvestrand.no>
wrote:

> Speaking only for myself ... I have a very strong dislike for double
> negatives.
>
> So "deactivateSender=false" and "unordered=false" gets a NO vote from me.
>
> On 02/17/2016 10:34 AM, 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.
> >
> > /Adam
> >
> > [1] https://github.com/w3c/webrtc-pc/pull/466
> > [2] https://github.com/w3c/webrtc-pc/pull/467
> > [3] https://github.com/w3c/webrtc-pc/pull/471
> > [4] http://heycam.github.io/webidl/#idl-dictionaries
> >
> >
>
>
>