-
Notifications
You must be signed in to change notification settings - Fork 53
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Principle: Write only one algorithm to accomplish a task. #562
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -24,6 +24,23 @@ Required IDs: using-http | |||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
Link Defaults: html (dfn) queue a task/in parallel/reflect | ||||||||||||||||||||||||||||||||
</pre> | ||||||||||||||||||||||||||||||||
<pre class="biblio"> | ||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||
"draft-ietf-oauth-sd-jwt-vc": { | ||||||||||||||||||||||||||||||||
"href": "https://datatracker.ietf.org/doc/html/draft-ietf-oauth-sd-jwt-vc", | ||||||||||||||||||||||||||||||||
"title": "SD-JWT-based Verifiable Credentials (SD-JWT VC)", | ||||||||||||||||||||||||||||||||
"status": "Internet-Draft", | ||||||||||||||||||||||||||||||||
"publisher": "IETF" | ||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||
"vc-data-model-1.1": { | ||||||||||||||||||||||||||||||||
"href": "https://www.w3.org/TR/vc-data-model-1.1/", | ||||||||||||||||||||||||||||||||
"title": "Verifiable Credentials Data Model v1.1", | ||||||||||||||||||||||||||||||||
"status": "REC", | ||||||||||||||||||||||||||||||||
"publisher": "W3C", | ||||||||||||||||||||||||||||||||
"deliveredBy": ["https://www.w3.org/groups/wg/vc"] | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
</pre> | ||||||||||||||||||||||||||||||||
<pre class="link-defaults"> | ||||||||||||||||||||||||||||||||
spec:css-cascade-5; type:dfn; text:inherit | ||||||||||||||||||||||||||||||||
spec:css2; type: property; text: line-height | ||||||||||||||||||||||||||||||||
|
@@ -3488,6 +3505,49 @@ While the best path forward may be to choose not to specify the feature, | |||||||||||||||||||||||||||||||
there is the risk that some implementations | ||||||||||||||||||||||||||||||||
may ship the feature as a nonstandard API. | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
<h3 id="multiple-algorithms">Write only one algorithm to accomplish a goal</h3> | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
When specifying how to accomplish a goal, write a single algorithm to do it, | ||||||||||||||||||||||||||||||||
instead of letting implementers pick between multiple algorithms. | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
It is very difficult to ensure that | ||||||||||||||||||||||||||||||||
two different algorithms produce the same results in all cases, | ||||||||||||||||||||||||||||||||
and doing so is rarely worth the cost. | ||||||||||||||||||||||||||||||||
Comment on lines
+3508
to
+3515
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
<div class="example" id="html-polyglot"> | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
[[html-polyglot inline]] was an attempt to define a common subset of both XHTML and HTML | ||||||||||||||||||||||||||||||||
that could be parsed into equivalent DOM trees | ||||||||||||||||||||||||||||||||
using either the [[HTML#the-xhtml-syntax|XHTML parsing]] | ||||||||||||||||||||||||||||||||
or [[HTML#syntax|HTML parsing]] algorithm. | ||||||||||||||||||||||||||||||||
Authors who tried to use this syntax tended to produce documents | ||||||||||||||||||||||||||||||||
that only worked with one of the two parsers. | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
<div class="example" id="json-ld-polyglot"> | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
The [[vc-data-model-1.1 inline obsolete]] and [[cid inline]] both endorse implementations | ||||||||||||||||||||||||||||||||
that use either JSON or JSON-LD to parse bytes into their data models. | ||||||||||||||||||||||||||||||||
Because JSON-LD provides many more ways | ||||||||||||||||||||||||||||||||
to assign properties to particular objects than JSON does, | ||||||||||||||||||||||||||||||||
these specifications had to add extra rules to both kinds of parsers | ||||||||||||||||||||||||||||||||
in order to ensure that each input document had exactly one possible interpretation. | ||||||||||||||||||||||||||||||||
[[vc-data-model-2.0 inline]] and [[draft-ietf-oauth-sd-jwt-vc inline]] fixed the problem | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Manu is right that these are completely different (and that they likely represent standardization failure, though the question of where the failure occurred might be contested). In a sense, it is OK that they are completely different (that they are in competition is potentially bad if they address the same use cases, but there is no risk that one might be mistaken for the other). I think that it would serve this example better to focus only on the CID case. |
||||||||||||||||||||||||||||||||
by defining different media types for JSON-LD vs JSON, | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This statement is incorrect. The VCDMv2 only has a single media type: Likewise, SD-JWT-VC only has one: Both specifications have parsing algorithms unique to their media types--and specific to their tasks. It remains unclear how these examples are "polyglot". |
||||||||||||||||||||||||||||||||
which means implementations are instructed to use exactly one of the algorithms. | ||||||||||||||||||||||||||||||||
The authors of [[cid inline]] have decided the extra specification work and testing | ||||||||||||||||||||||||||||||||
are acceptable prices to be able to allow both implementation strategies, | ||||||||||||||||||||||||||||||||
and they have partially mitigated the risks by explicitly stating that | ||||||||||||||||||||||||||||||||
if a single document can conformantly have multiple interpretations, | ||||||||||||||||||||||||||||||||
it's a specification bug. | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
</div> | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
Note: While [[rfc6838#section-6|structured suffixes]] define that | ||||||||||||||||||||||||||||||||
a document can be parsed in two different ways, | ||||||||||||||||||||||||||||||||
they do not violate this rule because the results have different data models. | ||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Isn’t the real difference here that the suffix parsing produces an intermediate result? i suspect that this is insufficient still, because it doesn’t really get at why suffix parsers exist. That is still somewhat contested, but my view is that intermediate results are rarely able to be processed meaningfully, so they are limited to use in diagnostic tools and the like. |
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
<h3 id="monkey-patching">Avoid monkey patching</h3> | ||||||||||||||||||||||||||||||||
A <dfn export>monkey patch</dfn> layers new functionality on top of an existing specification in a way that extends, overrides, or otherwise modifies the existing specification's behavior. | ||||||||||||||||||||||||||||||||
<a href="https://en.wikipedia.org/wiki/Monkey_patch">Monkey patching</a> <strong>is generally considered bad practice and should be avoided</strong> for the reasons listed below; but is sometimes unavoidable | ||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.