I generally agree with your sentiments, in that there are cases where you may want to use unused arguments for documentation purposes, and that there are cases where it is mistaken and/or misleading.
Here's an example of the former. https://github.com/fluid-project/infusion/blob/master/src/framework/core/js/Fluid.js#L171-L181 However, I feel that the latter is much more likely to be the case of most of our current instances. I'd rather guard against having misleading arguments and require ourselves to have to think an explicitly mark off cases where we want to leave in unused arguments. Furthermore, turning off this setting will likely not alert any unused variables, which opens us up to a lot of potential refactoring crumbs. I agree that we should avoid using the jshint ignore blocks, and opt for the single line wherever possible. Not only does the block style have the potential to ignore more errors, but in my experience it seems to also confuse jshint about variables and function declarations (may be fixed in newer versions of jshint). Thanks Justin On Apr 9, 2014, at 1:40 AM, Antranig Basman <[email protected]> wrote: > An odd issue that has come up in going through the jshint output on our > framework and component files relates to its treatment of unused function > parameters. Like the previous issue relating to bitwise operators, it is > somewhat systemic in that we have numerous locations in the code flagged by > this, but this time the underlying values seem a bit less clear-cut. > > It seems to me that in some situations, but by no means all, there is useful > documentation value in highlighting the presence of some arguments that are > always supplied but not always used - for example, the following situation > from InlineEdit.js seems fairly clear - > > fluid.inlineEdit.bindHoverHandlers = function (displayModeRenderer, > invitationStyle) { > var over = function (evt) { > > In this case, the argument "evt" is unused but is a clear hint to the reader > that this is a DOM or jQuery event handler. One from Tooltip.js: > > fluid.tooltip.idSearchFunc = function (idToContentFunc) { > return function (callback) { > > in this case this represents a somewhat peculiar feature of the tooltip > callback API that could easily be overlooked - and might return to use in a > future version of this callback, or an alternative implemented by an > integrator. > > Other cases are similar. > > It's clear that not ALL unused function arguments are actually benign - in > fact majority really do represent sloppy refactoring, and may even be a > positive evolution risk as signatures change whilst misleading unused > arguments get left behind. But it seems to me that there is some kind of > "hard core of benign unused parameters" case - > > I guess a possible summary that covers this case is - "function arguments > which form part of a stable signature, but which due to possibly accidental > details of a callback implementation are not currently used, but might become > used again with only a small change in requirements". It seems to me better > to be able to document such things in code rather than in comments - since > code is at least in theory machine readable, and will take exactly the same > form should the unused arguments become used. > > It seems undesirably noisy to bracket these entire blocks with a > jshint:ignore directive. If possible, we might mark these with the one-line > jshint ignore form. Alternatively, if there is a dedicated option for this, > we might get jshint to not complain about the entire class of "unused > function parameter" cases. > > Thoughts and comments? > > Cheers, > A > _______________________________________________________ > fluid-work mailing list - [email protected] > To unsubscribe, change settings or access archives, > see http://lists.idrc.ocad.ca/mailman/listinfo/fluid-work _______________________________________________________ fluid-work mailing list - [email protected] To unsubscribe, change settings or access archives, see http://lists.idrc.ocad.ca/mailman/listinfo/fluid-work
