25388 2014-04-18 08:10:04 +0000 Boolean arguments 2014-05-12 05:58:12 +0000 1 1 1 Unclassified Web Cryptography Web Cryptography API Document unspecified PC All RESOLVED WONTFIX P2 normal --- 1 annevk sleevi d public-webcrypto watsonm oldest_to_newest 104073 0 annevk 2014-04-18 08:10:04 +0000 Boolean arguments for methods are bad news. E.g. nobody ever remembers how addEventListener's third or XMLHttpRequest.prototype.open's third argument functions. See http://c2.com/cgi/wiki?CodeSmellMetrics It seems most of these methods could be designed to take a dictionary of sorts instead so the argument can be named and understood. 104636 1 sleevi 2014-04-29 00:09:22 +0000 (In reply to Anne from comment #0) > Boolean arguments for methods are bad news. E.g. nobody ever remembers how > addEventListener's third or XMLHttpRequest.prototype.open's third argument > functions. > > See http://c2.com/cgi/wiki?CodeSmellMetrics > > It seems most of these methods could be designed to take a dictionary of > sorts instead so the argument can be named and understood. I can appreciate this feedback, however, it does not seem to be terribly useful nor correct from a usability perspective. Please note that the core API objects already take a dictionary of arguments - the Algorithm dictionary (and it's sub-classes). This constitutes the bulk of the parameters one might specify to a given operation. Unless you're suggesting a dictionary-of-dictionaries, which itself seems a code-smell, I'm not sure how you see this being an improvement. In all of these methods, there is only a single boolean parameter - and it has the exact same meaning across all methods - "extractable". Possible interpretations of what you see as a "better" API include: dictionary DeriveKeyParams { AlgorithmIdentifier algorithm; // Really, "object" Key key; AlgorithmIdentifier derivedKeyType; // Really, "object" boolean extractable; Sequence<KeyUsage> keyUsages; // Instead of array }; Promise<any> deriveKey(DeriveKeyParams params); And it makes the calling convention go from window.crypto.subtle.deriveKey({ alg }, key, { derived alg }, false, [ "encrypt" ]); to window.crypto.subtle.deriveKey({ algorithm: { alg }, key: key, derivedKeyType: { derived alg }, extractable: false, keyUsages: [ "encrypt" ]); I cannot see how that's a better syntax - in part, it forgoes any possibility of conciseness, because all the dictionary attributes cannot be minified or elided. While I can understand holy wars have been raged over positional versus named arguments, that particular criticism doesn't seem to apply here, unless I'm missing something fundamental that has changed in how Web APIs are written. 104649 2 watsonm 2014-04-29 00:46:07 +0000 I interpreted the suggestion as being just to replace the extractable boolean with an enum { "extractable", "nonextractable" }. 104674 3 annevk 2014-04-29 10:12:36 +0000 I would not suggest to nest objects, that does indeed seem bad. Can it be a keyUsages parameter with values as per Mark's suggestion? The main problem is that if you look at a call such as window.crypto.subtle.deriveKey({ alg }, key, { derived alg }, false, [ "encrypt" ]); it is not clear what is going on. The same with the last argument being a length of sorts. I would need a much better understanding of the problem space to give more informed feedback as to what a better API would be. :-( 104712 4 sleevi 2014-04-29 21:00:37 +0000 (In reply to Anne from comment #3) > I would not suggest to nest objects, that does indeed seem bad. Can it be a > keyUsages parameter with values as per Mark's suggestion? > > The main problem is that if you look at a call such as > > window.crypto.subtle.deriveKey({ alg }, key, { derived alg }, false, [ > "encrypt" ]); > > it is not clear what is going on. The same with the last argument being a > length of sorts. A length? Not sure I follow. > > I would need a much better understanding of the problem space to give more > informed feedback as to what a better API would be. :-( I didn't take Mark's comment as suggesting it was a KeyUsage, but in fact a new enumeration. That is window.crypto.subtle.deriveKey({ alg }, key, {derived alg}, "extractable", [ "encrypt " ]); I took your reply as understanding it as window.crypto.subtle.deriveKey({alg}, key, {derived alg}, ["extractable", "encrypt"]); The issue here is that "extractable", as a key usage, doesn't have a 1:1 mapping to a given operation - that is "extractability" ties to both exportKey and wrapKey. Calling it "exportKey" doesn't conceptually fit, since you can extract a key via wrapping. Calling it "wrapKey" is off the table, since "wrapKey" is already a usage (indicating the given key can be used to wrap key, not that it is itself wrappable) As a style design, I can certainly understand why boolean positional arguments are bad when used in abundance - eg: foo(true, false, true); However, it seems a bit dogmatic to suggest that any boolean arguments are bad. 104714 5 d 2014-04-29 21:04:37 +0000 Are there any downsides to changing it from { true, false } to { "extractable", "nonextractable" }? There seems to be at least a slighty readability upside. 104716 6 d 2014-04-29 21:06:50 +0000 I guess the downside would be that, for objects in the spec with the `.extractable` property, you then would either: - Have them return true/false, making them inconsistent with the methods under question - Have them return "extractable"/"nonextractable", which seems very error prone especially since both are truthy (e.g. `if (o.extractable) { ... }` will always pass). With this in mind I think the boolean argument, while not very good for readability, is probably the best option. 105677 7 sleevi 2014-05-12 05:58:12 +0000 Closing, per Comment 6