-1
\$\begingroup\$

I'm currently working on an enormous, enterprise level project. There are about 130 other devs on said project, and I aim to document my code in the clearest way, so others may easily see what I'm doing and why. I'm using JSDoc style comments to summarize my functions, but sometimes I feel the need to break it down to a more granular level.

A particular method, using a decently obtuse function chain, comes to mind. To provide a bit of background... this method takes an integer which is the sum of various powers of two, where each power represents a string "rule". It derives a subset of rules from master list using this sum.

What is a cleaner, more straightforward way to document "steps" within a function, giving line-by-line descriptions if necessary?

...
},
intToRuleset: function(subset) { // ex: intToRuleset(40);
    var rules = this.rules, // the master list of rules
        mask = (subset) // 40
            .toString(2) // '101000'
            .split('')// ['1','0', '1','0','0','0']
            .reverse(); // ['0','0','0','1', '0','1']

    return rules.filter(function(a, i) {
        return +mask[i];
    });
},
...

As you can see, I'm only crappily explaining what I'm doing; it seems mostly redundant to me as I'm only providing the result of each function... any fool can guess that the reverse of ['foo', 'bar'] is ['bar', 'foo'].

What I want to do is describe both what I'm doing and why/my thought process.

I've tried a few things: verbose inline commentary both above and below lines, labeled each with a number as a key/legend sol'n, etc. I think all attempts have been ugly and made the code harder to read overall.

...
},
intToRuleset: function(subset) { // ex: intToRuleset(40);
    var rules = this.rules, // the master list of rules

        mask = (subset) // 40
            /* 1 - convert into base 2 to form mask -- we'll exploit
                   truthy/false-y 1s and 0s to filter out the associated               
                   rules. */
            .toString(2) // '101000'
            /* 2 - split into array so it can be reversed */
            .split('')// ['1','0', '1','0','0','0']
            /* 3 - reversing the "mask" array so it aligns with  */
            .reverse(); // ['0','0','0','1', '0','1']

    return rules.filter(function(a, i) {
        /* 4 - coerce string into number into "boolean";
           if 1, rule stays; otherwise, it's filtered out. 
         */
        return +mask[i]; 
    });
       // example return:
       // ['034 - 50000lb waffle !=  fuzzball', 
       //  '222 - steeple-people === dentists]
},
...

What's a better way to comment/document steps like this?

\$\endgroup\$
  • \$\begingroup\$ Might as well tell them to use LightTable, and interpret line by line. So many comments detract from the code IMO. \$\endgroup\$ – elclanrs Jan 20 '15 at 19:07
  • 3
    \$\begingroup\$ I agree with @Jamal: we need to see more context to be able to review this code properly. What other functions are there in the object? Is this intended to be a "public" or "private" method? What is the purpose of these rules, and what will the rules be operating on? I suspect that simply rewriting your line-by-line comments would be giving you bad advice, but we can't give you good advice unless you show us more. ("The code itself is not particularly relevant to the question" is a self-defeating attitude when posting to a site called Code Review!) \$\endgroup\$ – 200_success Jan 20 '15 at 20:08
  • 3
    \$\begingroup\$ I'm not looking for pointers on the code itself - then your question is off-topic for this site. Please see our help center for all the details; feel free to ask on meta or come to The 2nd Monitor if you have any questions regarding the scope of the site. \$\endgroup\$ – Mathieu Guindon Jan 20 '15 at 20:26
  • 2
    \$\begingroup\$ @Todd Please see our six on-topic questions. 1) Is code included directly in my question? 2) Am I an owner or maintainer of the code? 3) Is it actual code from a project rather than pseudo-code or example code? 4) Do I want the code to be good code? 5) To the best of my knowledge, does the code work as intended? 6) Do I want feedback about any or all facets of the code? We currently feel that your answer to questions 3 and 6 might not be a clear "Yes". \$\endgroup\$ – Simon Forsberg Jan 20 '15 at 20:58
  • 1
    \$\begingroup\$ There is no trolling. The quality of the advice you get is commensurate to the quality of your question. I had put your question on hold in an attempt to try to get you to ask a better question, that's all. \$\endgroup\$ – 200_success Jan 20 '15 at 21:27
-1
\$\begingroup\$

You could make two versions of code- one without comments and one with comments. Like we have jQuery and jQuery-min.

I am not sure if this is really good practice. Maintenance of two files can also be little difficult.

\$\endgroup\$
  • 4
    \$\begingroup\$ Nobody "maintains" jquery-min; it's simply produced by passing the "full" jQuery file through a minifier to save bandwidth \$\endgroup\$ – Flambino Jan 20 '15 at 19:36
  • 1
    \$\begingroup\$ I appreciate the input; it's generally good advice. It doesn't answer the question, though. We're already minifying our code, and the code in question IS the DEVELOPMENT vers only. The codebase is massive and confusing enough, so I'd worsen the issue by creating a separate file. \$\endgroup\$ – Todd Jan 20 '15 at 19:37
  • \$\begingroup\$ I did not downvote this answer. In fact, you've been the only person to offer any real advice, rather than troll me. upv'd \$\endgroup\$ – Todd Jan 20 '15 at 21:17
  • \$\begingroup\$ hell, I'll accept it as right, despite it not being what I was looking for. At least it's legitimate advice. (Inappropriate language removed) thanks for your honest attempt to help. \$\endgroup\$ – Todd Jan 20 '15 at 21:19
  • 1
    \$\begingroup\$ Civility is required on this site at all times. Your inappropriate comment has been redacted. \$\endgroup\$ – 200_success Jan 20 '15 at 21:25
3
\$\begingroup\$

You shouldn't write line-by-line comments, for three reasons:

  1. Line-by-line comments merely restate the code without adding insight. As such, they are clutter — more harmful than helpful.
  2. You don't try to understand a large application by tracing every line of code. Other developers want to trust that your method does what it claims to do, and avoid reading your code and comments. So, make it clear what your method does. If they ever need to verify that it does what it claims, then they'll read the code anyway.
  3. There was way too much code to begin with. This method should have been a one-line expression.

In addition, the naming of the method is a bit off, in my opinion. Here's my recommendation:

/**
 * Returns a subset of the master list of rules.
 *
 * Example: Given rules
 *     ['alpha dog == Fido', 'beta test != no', 'gamma radiation == danger']
 *
 * rulesetForSelector(6) would return
 *     ['beta test != no', 'gamma radiation == danger']
 *
 * @param {number} bitmask - A integer selecting the rules to include, where the
 *        least-significant bit corresponds to the first rule.
 */
rulesetForSelector: function(bitmask) {
    return this.rules.filter(function(rule, i) {
        return (bitmask >> i) & 1;
    });
},
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.