cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Andrew Grieve <agri...@google.com>
Subject Type checking in Cordova JS plugins
Date Thu, 15 Nov 2012 20:15:14 GMT
There's very little consistency when it comes to checking params in plugin
code.

globalization.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/globalization.js;h=a57062ce8b1e0cc95dde54f8ca600f6ee4876bfd;hb=HEAD>:
checks every args. logs errors and returns without calling errback, does
not allow missing errCB.

resolveLocalFileSystemURI.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/resolveLocalFileSystemURI.js;h=c83b0aac3704c46782c3e19876afb180c32fc081;hb=HEAD>:
Allows missing successCB and errCB. Sanity checks URL has a colon in it.

notification.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/notification.js;h=fbf9046a0e03bb6647219d7201b23f26f4d47084;hb=HEAD>:
doesn't check types at all

contacts.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/contacts.js;h=2f5a51a20c8ad14b96c4ef935a28686a9bd87eca;hb=HEAD>:
Throws TypeError if missing successCB, allows missing errCB

compass.js<https://git-wip-us.apache.org/repos/asf?p=incubator-cordova-js.git;a=blob;f=lib/common/plugin/compass.js;h=c97c1588bf69d2c94cde64736345a7d8b1dc32fe;hb=HEAD>:
Logs and returns if callbacks are of wrong type. success required, error
optional.


So, the things to agree upon are:

1. Should we check types or not?
2. Success / error callbacks : optional or not?
3. When type errors happen, log & return, call errBack, or throw TypeError?



For some extra inspiration, I played around with what it would look like to
at least factor out type checking code. Have a look at the
module<https://github.com/agrieve/incubator-cordova-js/blob/argscheck/lib/common/argscheck.js>and
it's
tests<https://github.com/agrieve/incubator-cordova-js/blob/argscheck/test/test.argscheck.js>.
Also, here's<https://github.com/agrieve/incubator-cordova-js/blob/argscheck/lib/common/plugin/globalization.js>what
it looks like being used in globalization.js.


Now, here's my thoughts for the three questions
1. Should we check types or not?
  - I think it is useful since incorrect args end up in exec() calls and
are therefore harder to debug when they are wrong.
  - Perhaps instead of checking types, we add a helper for coercing types?
This may only apply to strings / numbers though...
  - I also kind of like the idea of being able to turn off type checking
for release mode. Having a common function to do the type checking would
allow us to turn checking on/off for performance.

2. Success / error callbacks : optional or not?
  - I'm not positive we need to make these all consistent
  - If anything, I'd say we'd want them to be optional.

3. When type errors happen, log & return, call errBack, or throw TypeError?
  - TypeError is my preference here. This matches what browser APIs do when
it doesn't like params.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message