4

I have this method that is calculating a total, and part of it is giving a warning in JS Lint. We're trying to get cleaner JS Lint inspections at work, so I want to see if there's a rational way to get around this that I'm not thinking of.

calculateTotal = function() {
    var hours = parseFloat($hours.val());
    var rate = parserFloat($rate.val());
    var total = '';

    if (!isNaN(hours) && !isNaN(rate)) {
        // This throws the error.
        total = (rate * hours).toFixed(2);
    }

    $total.val(total);
}

I can avoid the message if I do the following:

total = rate * hours;
total = total.toFixed(2);

It's a little too verbose for me to just jump at it, but it may be the best bet.

I checked out this question, and considered doing Number(rate * hours).toFixed(2), but that's (marginally) less performant, plus it would be a bad precedent to start with all of the warnings about using String() as stated in response to the accepted answer there.

This could be moot if my above attempt is the best way to get JS Lint to stop complaining, but I would like to hear from other people.

Community
  • 1
  • 1
krillgar
  • 12,596
  • 6
  • 50
  • 86
  • I don't see anything wrong with your original code. Calling `toFixed()` on that expression looks fine to me. By the way, you'd want add a colon after the assignment of your function. – Nayuki Apr 12 '16 at 19:05
  • 4
    I'd strongly recommend finding an alternative to JSLint, and it's outright silly rules warning about perfectly valid JavaScript. Consider ESLint, JSCS, or even JSHint. – Alexander O'Mara Apr 12 '16 at 19:05
  • @Nayuki Yeah, it is there in my code. I just typed up a quick demo, and it was dumb omission on my part. If I have another reason to make an edit to my question, I'll add that. Thanks! – krillgar Apr 12 '16 at 19:23
  • @AlexanderO'Mara Do those two have Visual Studio plugins? That's how we're doing our analysis. I have typically just pasted my code into jslint.com, and received at most one or 2 warnings out of a thousand lines of code. I'm on assignment right now at a client, and it was their call to have this add-in. If there is a better alternative, I will definitely make the suggestion. – krillgar Apr 12 '16 at 19:24
  • I don't know, I'm happy to say I have 0 Microsoft products installed on my system, and have yet to find one worth installing. This question might be helpful: [Integrate EsLint with visual studio 2015](http://stackoverflow.com/questions/36034652/integrate-eslint-with-visual-studio-2015) – Alexander O'Mara Apr 12 '16 at 19:29
  • 1
    @AlexanderO'Mara Good heavens. Sorry to vent on you on this question, but **if it's a JSLint question, give a JSLint answer**. I don't spam up the ESLint & JSHint questions with, "*You really should take out all of your dimestore opinions about JavaScript and use a linter with strong opinions to more universally standardize your codebase."* Commenting "your tool stink0rz" is like saying, "Stop using JavaScript. VBScript rulez!1!" It's just opinion. Crockford's rarely demonstrably *wrong*; often, folks who make these comments don't take the time to understand what he recommends or why. – ruffin Apr 12 '16 at 19:29
  • @ruffin *if it's a JSLint question, give a JSLint answer* I didn't give an answer, I gave a comment. And I didn't says JSHint sucks (I'll happily say that about VBScript though :) ), I just said I recommend alternative tools which many consider better (I'm not aware of any major projects using JSLint). I am aware of many of the reason why JSLint complains, but I find many of the reasons it complains actively harm JavaScript more than they help. My main issue with it though is the general lack of flexibility. – Alexander O'Mara Apr 12 '16 at 19:34
  • @AlexanderO'Mara Your comment's got nothing to do with how to solve *this* post's question w/ a JSLint solution. [Take another look](http://stackoverflow.com/help/privileges/comment): Comment to ask for clarification, to improve the post, to link to a related question. *Don't comment* to start a 2ndary discussion; use chat instead. Again, sorry to unload on you specifically, but saying JSLint has "outright silly rules" @ "perfectly valid JavaScript" is not to understand what a linter does. Show me where JSLint "actively harms Javacript" & I'll show you someone who doesn't understand the lang. – ruffin Apr 12 '16 at 21:07
  • @ruffin I couldn't find "recommending alternative solutions" under "shouldn't comment" and I would say that "perfectly valid JavaScript" is relevant information. The purpose of a linter is generally to flag bad or potentially bad code, based on some criteria. In the case of JSLint, that criteria is not very objective (whatever Crockford's chooses basically). In the realm of ESLint, one can choose what features to check. – Alexander O'Mara Apr 12 '16 at 21:31
  • @AlexanderO'Mara /sigh [I'm ending this transmission.](https://www.youtube.com/watch?v=XQduN315etk&t=23m43s) (The irony, of course, is that I think krillgar's found a bug in the new JSLint.) – ruffin Apr 12 '16 at 21:34
  • @ruffin 2 samples of harmful warnings I've seen by JSLint (which I assume are still present). **1.** Disallows the use of `new` completely (even before `Object.create`, which isn't really a replacement). The `new` keyword is a fundamental feature of JavaScript prototypical nature. Just because some people make mistakes or forget to include it, is not a reason to never use it (but it would be a great time for linters to show warnings when omitted). **2.** It warns that some regex patterns are a security risk, because they may match too much. Magic warning like this give false sense of security. – Alexander O'Mara Apr 12 '16 at 21:35
  • @krillgar Welp, [not a bug](https://plus.google.com/116155444999910002488/posts/EdhyU8tYn1Y). I'd use `$total.val(total.toFixed(2));`, fwiw. – ruffin Apr 12 '16 at 22:45
  • @ruffin Yeah, I don't consider it a bug due to the potential problems that were discussed in the question I linked. However, I do really like you fix. If you want to make it an answer, I'll strongly consider that as the accepted answer. Just give it until tomorrow to see if someone else has an idea. – krillgar Apr 12 '16 at 22:47

1 Answers1

2

TL;DR

JSLint is going to force you to move the toFixed() from behind of the parentheses. I'd suggest the least annoying place to move it is in the $total.val(total) assignment.

This lints as-is on JSLint.com:

/*jslint white:true, browser:true */
/*global $hours, $rate, $total */

var calculateTotal = function() {
    "use strict";
    var hours = parseFloat($hours.val());
    var rate = parseFloat($rate.val());
    var total;

    if (!isNaN(hours) && !isNaN(rate)) {
        // This throws the error.
        total = rate * hours;
    }

    $total.val(total.toFixed(2));  // moved `toFixed` to here
};

A little longer...

I tried it against the most recent version of JSLint, and it's borking at left_check in JSLint's code, here:

function left_check(left, right) {

// Warn if the left is not one of these:
//      e.b
//      e[b]
//      e()
//      identifier

    var id = left.id;
    if (
        !left.identifier &&
        (
            left.arity !== "binary" ||
            (id !== "." && id !== "(" && id !== "[")
        )
    ) {
        warn("unexpected_a", right);
        return false;
    }
    return true;
}

left is essentially (rate & hours) and right is . with toFixed the next token in this case.

As dangerous as it is to assume code function from comments, I think the comments tell us where JSLint is coming from -- It wants methods called only on objects, not on operations, including type coercions that often occur inside of them. It pretty much has to let you make "fluent" calls, where you chain methods, and the only valid things that can have method calls are...

  • an object: e
  • A property of an object: e.b
  • A property in a collection: e[key]
  • The return value of a function: e()

Just to double check, since your construction used to work in "old JSLint" (the last version before JSLint for ES6), I asked Douglas Crockford. He's pretty terse, but he did confirm JSLint is working as intended.

crockford on paren method

Sorry I can't be more help there. I think there are places where (someExpression).someMethod() is expedient, but understand where JSLint's coming from too. If you're going to have the potential for type coercion, coerce explicitly.

Interesting question; thanks for asking.

Community
  • 1
  • 1
ruffin
  • 16,507
  • 9
  • 88
  • 138
  • 1
    You're plenty of help. Where he's coming from does make a ton of sense. This is one of those situations where as a human, we know it won't apply, but when you're writing a program to do something generically, you can't **guarantee** that it will always be that way, so err on the side of caution. – krillgar Mar 30 '17 at 16:24