0

hullo all,

jslint is mad at me. i'm making a function within a loop, but i'm also not quite sure how to fix it, since it seems i would need to curry the result such that the function matches the signature expected by request for a callback as well as perform variable capture correctly or perform some other javascript devilry to simplify that.

edit: the code as it is works fine. i'd just like to know how to make it so the linter isn't made at me anymore!

the section of code looks like the following:

function func(cb) {
    request({ params }, (error, response) => {
        const devices = response.body;
        let completedCounter = 0;
        for (const device of devices) {
        request({ params }, (err, response) => {
            if (!err && response.statusCode === 200) {
            completedCounter += 1;
            if (completedCounter === devices.length) {
                cb(null, "message here");
            }
            } else {
            cb(err, "message here");
            }
        });
        }
    });
}

sorry if my terminology isn't typical javascript terminology- i'm a confused c++/python/lisp programmer outside his comfort zone!

the linter output is:

39:10  error  Don't make functions within a loop  no-loop-func
calben
  • 1,328
  • 3
  • 18
  • 33
  • Substituting a valid object for `{ params } ` and changing `const device of devices` to `let device of devices` allows the function to compile without errors. After doing this and using a valid parameter obect, does the code work? – traktor Oct 24 '16 at 23:05
  • oh the code works great as it is. it's just the linter doesn't like the fact that i'm making a request in a loop because technically when a make the request i'm building a function inside of it! i'll make that clearer in the question :-) – calben Oct 24 '16 at 23:22
  • Looks like your linter is not ES6 compatible. Maybe try eslint instead? – Bergi Oct 24 '16 at 23:28
  • @Traktor53 both property shorthands and `const` declarations are valid code that don't need to be changed to compile it without errors. – Bergi Oct 24 '16 at 23:29
  • i've updated the post to include the lint output! should it not be concerned that i'm making a function in a loop? – calben Oct 24 '16 at 23:38
  • 1
    @calben Not really, it is what you want to do here. To satisfy your linter, you'd have to call a function in the loop body (that makes one request and creates one callback each iteration). But that's quite outdated, and you've already mitigated the most concerning problem with closures inside loops by using `const` instead of `var`. – Bergi Oct 24 '16 at 23:43
  • @Bergi The object shorthand is fine (thanks) but I'm getting an error for `const device of devices` in Firefox: "SyntaxError: missing = in const declaration". – traktor Oct 25 '16 at 00:18
  • @Traktor53 Oh my, [it's a known bug](http://stackoverflow.com/q/39044803/1048572). Upgrade to FF51 :-) – Bergi Oct 25 '16 at 00:29

1 Answers1

1

I'm not exactly sure what you're trying to do in the code, as there are parts missing (or parts that are gratuitous, which I've commented or stubbed out, below), but this lints in the most recent version of JSLint.

/*jslint node, es6, for, white */

var request = require("request");

function func(cb) {
    "use strict";

    var params = {};

    // See https://plus.google.com/108103301822680819489/posts/ZWBUMhYGcrH
    request(params, function (ignore, response) {
        const devices = response.body;
        var completedCounter = 0;

        Object.keys(devices).forEach(function(/*k*/) {
            // var device = devices[k]; // `device` is unused, so we need neither it nor `k`, above
            var params2 = {};

            request(params2, function (err, response2) {
                if (!err && response2.statusCode === 200) {
                    completedCounter += 1;
                    if (completedCounter === devices.length) {
                        cb(null, "message here");
                    }
                } else {
                    cb(err, "message here");
                }
            });
        });
    });
}

func(); // Needs to be used or exported somehow.

Farts/Arrow Notation in JSLint

JSLint does want a "fart" (arrow notation) to return in a single line. Otherwise, JSLint prefers you use function ( notation to make what you're doing more clear.

Check the discussion here, at the JSLint discussion group. Crockford demonstrates what I suggest, above, and then Lee Chase adds...

Arrow functions have a problem with { }, mainly is it a multiline function or an object.

As it happens your function returns nothing as multiline arrow functions need a return.

That makes some sense. And the OP at that link makes precisely the mistake JSLint is trying to help you avoid. That is, the stylistic error did in fact (in that case) cause a functional one.

Your "function in a loop" issue

Using forEach with Object.keys, another set of JSLint recommendations (Object.keys and for) helps eliminate the overhead of redeclaring functions as well. You don't, as a rule, want to use for (x in y).

Though why JSLint does not consider for... of to be a "good part" of ES6, I don't know.

ruffin
  • 16,507
  • 9
  • 88
  • 138