- From: Domenic Denicola <notifications@github.com>
- Date: Fri, 25 Aug 2017 03:00:13 +0000 (UTC)
- To: heycam/webidl <webidl@noreply.github.com>
- Cc: Subscribed <subscribed@noreply.github.com>
- Message-ID: <heycam/webidl/pull/426/review/58558635@github.com>
domenic commented on this pull request.
SO GOOD!!
A number of nits. But thanks so much for working on this.
> @@ -3331,6 +3331,207 @@ be the same.
</div>
+<h5 id="idl-overloading-vs-union">Overloading vs. union types</h5>
+
+<i>This section is informative.</i>
+
+For specifications defining IDL [=operations=], it may seem that [=overloaded|overloads=] and a
+combination of [=union types=] and [=optional argument=] have some feature overlap.
+
+It is first important to note that [=overloaded|overloads=] may have different behaviors than
+[=union types=] or [=optional arguments=], and one <em>cannot</em> be fully defined using the
+other (unless, of course, additional prose is provided, which can defeat the purpose of the Web IDL
+type system). For example, given the {{CanvasDrawPath/stroke()}} operations defined on the
s/given/consider, as otherwise it sounds like the first part of a sentence that is then left hanging.
> @@ -3331,6 +3331,207 @@ be the same.
</div>
+<h5 id="idl-overloading-vs-union">Overloading vs. union types</h5>
+
+<i>This section is informative.</i>
There's some way of wrapping a section to say everything inside it is non-normative so that autolinks like `{{CanvasDrawPath}}` don't generate normative references, and so you get warnings when you try to use normative keywords. It appears to be undocumented. @tabatkins?
> @@ -3331,6 +3331,207 @@ be the same.
</div>
+<h5 id="idl-overloading-vs-union">Overloading vs. union types</h5>
+
+<i>This section is informative.</i>
+
+For specifications defining IDL [=operations=], it may seem that [=overloaded|overloads=] and a
+combination of [=union types=] and [=optional argument=] have some feature overlap.
+
+It is first important to note that [=overloaded|overloads=] may have different behaviors than
+[=union types=] or [=optional arguments=], and one <em>cannot</em> be fully defined using the
+other (unless, of course, additional prose is provided, which can defeat the purpose of the Web IDL
+type system). For example, given the {{CanvasDrawPath/stroke()}} operations defined on the
+{{CanvasDrawPath}} interface [[HTML]]:
+
+<pre highlight="webidl">
+ interface CanvasDrawPath {
Maybe call it `CanvasDrawPathExcerpt` or something? Especially because the below version is not CanvasDrawPath at all.
> +It is first important to note that [=overloaded|overloads=] may have different behaviors than
+[=union types=] or [=optional arguments=], and one <em>cannot</em> be fully defined using the
+other (unless, of course, additional prose is provided, which can defeat the purpose of the Web IDL
+type system). For example, given the {{CanvasDrawPath/stroke()}} operations defined on the
+{{CanvasDrawPath}} interface [[HTML]]:
+
+<pre highlight="webidl">
+ interface CanvasDrawPath {
+ void stroke();
+ void stroke(Path2D path);
+ };
+</pre>
+
+Per the ECMAScript language binding, calling <code>stroke(undefined)</code> on a object implementing
+{{CanvasDrawPath}} would throw a {{TypeError}}. However, if the operations were instead defined with
+[=optional arguments=] and merged into one:
comma here instead of colon, I think
> + void stroke();
+ void stroke(Path2D path);
+ };
+</pre>
+
+Per the ECMAScript language binding, calling <code>stroke(undefined)</code> on a object implementing
+{{CanvasDrawPath}} would throw a {{TypeError}}. However, if the operations were instead defined with
+[=optional arguments=] and merged into one:
+
+<pre highlight="webidl">
+ interface CanvasDrawPath {
+ void stroke(optional Path2D path);
+ };
+</pre>
+
+The [=overload resolution algorithm=] would treat the |path| argument as not present given the same
lowercase "the" since it's continuing a sentence
> +
+It is first important to note that [=overloaded|overloads=] may have different behaviors than
+[=union types=] or [=optional arguments=], and one <em>cannot</em> be fully defined using the
+other (unless, of course, additional prose is provided, which can defeat the purpose of the Web IDL
+type system). For example, given the {{CanvasDrawPath/stroke()}} operations defined on the
+{{CanvasDrawPath}} interface [[HTML]]:
+
+<pre highlight="webidl">
+ interface CanvasDrawPath {
+ void stroke();
+ void stroke(Path2D path);
+ };
+</pre>
+
+Per the ECMAScript language binding, calling <code>stroke(undefined)</code> on a object implementing
+{{CanvasDrawPath}} would throw a {{TypeError}}. However, if the operations were instead defined with
"would attempt to call the second overload, yielding a TypeError since undefined cannot be converted to a Path2D".
> +
+Per the ECMAScript language binding, calling <code>stroke(undefined)</code> on a object implementing
+{{CanvasDrawPath}} would throw a {{TypeError}}. However, if the operations were instead defined with
+[=optional arguments=] and merged into one:
+
+<pre highlight="webidl">
+ interface CanvasDrawPath {
+ void stroke(optional Path2D path);
+ };
+</pre>
+
+The [=overload resolution algorithm=] would treat the |path| argument as not present given the same
+ECMAScript language code <code>stroke(undefined)</code>, and not throw any exceptions.
+
+Additionally, there are semantic differences as well. [=Overloaded=] operations are designed to map
+well to C++ overloading at the language binding, and are usually a better fit for operations with
Not sure this is "at the language binding", but the sentence is true and this is good to point out...
> +<pre highlight="webidl">
+ interface CanvasDrawPath {
+ void stroke(optional Path2D path);
+ };
+</pre>
+
+The [=overload resolution algorithm=] would treat the |path| argument as not present given the same
+ECMAScript language code <code>stroke(undefined)</code>, and not throw any exceptions.
+
+Additionally, there are semantic differences as well. [=Overloaded=] operations are designed to map
+well to C++ overloading at the language binding, and are usually a better fit for operations with
+more substantial differences in what they do given arguments of different types. [=Union types=], in
+contrast, are usually used in the sense that "any of the types would work in about the same way".
+
+That being said, we offer the following recommendations and examples in case of difficulties to
+determine what Web IDL language feature to use.
Colon here
> +
+Additionally, there are semantic differences as well. [=Overloaded=] operations are designed to map
+well to C++ overloading at the language binding, and are usually a better fit for operations with
+more substantial differences in what they do given arguments of different types. [=Union types=], in
+contrast, are usually used in the sense that "any of the types would work in about the same way".
+
+That being said, we offer the following recommendations and examples in case of difficulties to
+determine what Web IDL language feature to use.
+
+* When the operation must return values of different types for different argument types,
+ [=overloaded|overloading=] is almost always a better choice.
+
+ Suppose there is an operation <code class="idl">calculateWithType()</code> that accepts a single
+ argument of either {{long}}, {{DOMString}}, or <code class="idl">CalculatableInterface</code>
+ (an interface type) types, and returns a value of the same type as its argument. It would seem
+ to be clearer to write the IDL fragment using [=overloaded=] operations as
"it would be clearer", I think
> + <pre highlight="webidl">
+ partial interface CSS {
+ static boolean supports(CSSOMString property, CSSOMString value);
+ static boolean supports(CSSOMString conditionText);
+ };
+ </pre>
+
+ Using [=optional arguments=] one can rewrite the IDL fragment as follows:
+
+ <pre highlight="webidl">
+ partial interface CSS {
+ static boolean supports(CSSOMString propertyOrConditionText, optional CSSOMString value);
+ };
+ </pre>
+
+ It can be observed that, even though the IDL is shorter, two distinctively different concepts
Remove "It can be observed that" clause
> + };
+ </pre>
+
+ Using [=optional arguments=] one can rewrite the IDL fragment as follows:
+
+ <pre highlight="webidl">
+ partial interface CSS {
+ static boolean supports(CSSOMString propertyOrConditionText, optional CSSOMString value);
+ };
+ </pre>
+
+ It can be observed that, even though the IDL is shorter, two distinctively different concepts
+ are conflated in the first argument. This makes the second version remarkably less readable than
+ the first.
+
+ Another consideration is that many specification preprocessors allow linking to two different
I think this paragraph mentions specification preprocessors when that's not the primary issue. The real issue is clarity of speccing and duplicating the overload resolution logic inside your steps. I'd just say something about how specifications are easier to write and read in this case.
> + method, when called, must run these steps:
+
+ 1. …
+
+ ----
+
+ The <code class="idl">supports(<var ignore>conditionText</var>)</code> method, when called,
+ must run these steps:
+
+ 1. …
+
+ </div>
+
+ Yet using optional argument, the specification author must use more boilerplate-style text to
+ effectively replicate the overload logic. If the two overloads have little to no shared parts,
+ it is better to leave overload resolution to the IDL mechanism.
Let's move the conclusion sentence after this next algorithm.
Should "overload logic" link to the algorithm?
> + void foo(Node? arg);
+ };
+ </pre>
+
+ Using the ECMAScript language binding, calling <code>foo(undefined)</code> and
+ <code>foo(null)</code> would both run the steps corresponding to the <code
+ class="idl">foo(|arg|)</code> operation, with |arg| set to null, while <code>foo()</code> alone
+ would go to the first overload. This may be a surprising behavior for many API users. Instead,
+ it is recommended to use an optional argument, which would categorize both <code>foo()</code>
+ and <code>foo(undefined)</code> as "|arg| is not present".
+
+ <pre highlight="webidl">
+ interface A {
+ void foo(optional Node? arg);
+ };
+ </pre>
Maybe a conclusion of "In general, optionality is best expressed using the optional keyword, and not using overloads".
> + Using the ECMAScript language binding, calling <code>foo(undefined)</code> and
+ <code>foo(null)</code> would both run the steps corresponding to the <code
+ class="idl">foo(|arg|)</code> operation, with |arg| set to null, while <code>foo()</code> alone
+ would go to the first overload. This may be a surprising behavior for many API users. Instead,
+ it is recommended to use an optional argument, which would categorize both <code>foo()</code>
+ and <code>foo(undefined)</code> as "|arg| is not present".
+
+ <pre highlight="webidl">
+ interface A {
+ void foo(optional Node? arg);
+ };
+ </pre>
+
+When the case fits none of the categories above, it is up to the specification author to choose the
+style, since it is most likely that either style would sufficiently and conveniently describe the
+intended behavior. However, it should be noted that the definition and conversion algorithms of
remove "it should be noted"
> + class="idl">foo(|arg|)</code> operation, with |arg| set to null, while <code>foo()</code> alone
+ would go to the first overload. This may be a surprising behavior for many API users. Instead,
+ it is recommended to use an optional argument, which would categorize both <code>foo()</code>
+ and <code>foo(undefined)</code> as "|arg| is not present".
+
+ <pre highlight="webidl">
+ interface A {
+ void foo(optional Node? arg);
+ };
+ </pre>
+
+When the case fits none of the categories above, it is up to the specification author to choose the
+style, since it is most likely that either style would sufficiently and conveniently describe the
+intended behavior. However, it should be noted that the definition and conversion algorithms of
+[=union types=] are simpler to implement and reason about, and can discourage implementation
+mistakes.
maybe add "and so should be your default choice"
> + <code>foo(null)</code> would both run the steps corresponding to the <code
+ class="idl">foo(|arg|)</code> operation, with |arg| set to null, while <code>foo()</code> alone
+ would go to the first overload. This may be a surprising behavior for many API users. Instead,
+ it is recommended to use an optional argument, which would categorize both <code>foo()</code>
+ and <code>foo(undefined)</code> as "|arg| is not present".
+
+ <pre highlight="webidl">
+ interface A {
+ void foo(optional Node? arg);
+ };
+ </pre>
+
+When the case fits none of the categories above, it is up to the specification author to choose the
+style, since it is most likely that either style would sufficiently and conveniently describe the
+intended behavior. However, it should be noted that the definition and conversion algorithms of
+[=union types=] are simpler to implement and reason about, and can discourage implementation
"simpler ... than overloads, and can"
> @@ -3331,6 +3331,207 @@ be the same.
</div>
+<h5 id="idl-overloading-vs-union">Overloading vs. union types</h5>
+
+<i>This section is informative.</i>
+
+For specifications defining IDL [=operations=], it may seem that [=overloaded|overloads=] and a
+combination of [=union types=] and [=optional argument=] have some feature overlap.
optional argument_s_
--
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/heycam/webidl/pull/426#pullrequestreview-58558635
Received on Friday, 25 August 2017 03:00:37 UTC