4
\$\begingroup\$

I've been writing JavaScript for a couple years but nobody's ever really looked at my code.

This is a small library I wrote to check code modularity, w/ input like avg-lines . -name '*.js' (the arguments are passed straight to find) you might get:

Checked 20 files (1615 LOC):
  80% of files contain > 137 LOC
  20% of files contain 67.68% of the total LOC
  The largest file (./dist/redux.js) contains 638 LOC

index.js

import {exec} from 'child_process'

const i80 = arr => Math.ceil(arr.length * 0.8)
const sum = arr => arr.reduce((pv, [v]) => pv + v, 0)
const insertTypeOption = args =>
  /('?[-\|])/.test(args) ? args.replace(/('?[-\|])/, '-type f $&') : `${args} -type f`
const generateCommand = args => `find ${insertTypeOption(args)} | xargs wc -l`

export default args => new Promise((resolve, reject) => {
  const process = exec(generateCommand(args))
  const files = []
  const interval = setInterval(() => console.log(`checked ${files.length} files`), 3000)
  process.stderr.on('data', ::console.error)
  process.stdout.on('data', input =>
    files.push(
      ...input.split('\n').map(l =>
        l && l.match(/^ *(\d+) +(.+)$/)).filter(v => v).
        map(([, val, file]) => [parseInt(val, 10), file]).
        filter(([, filename]) => filename !== 'total')))
  process.on('close', exit => {
    clearInterval(interval)
    if (exit) reject(exit)
    files.sort(([v0], [v1]) => v0 - v1)
    if (!files.length) reject(`No files matched \`${generateCommand(args)}\``)
    else {
      resolve({
        numFiles: files.length,
        numLines: sum(files),
        avgLines: files[i80(files)][0],
        linesInequality: sum(files.slice(i80(files))) / sum(files),
        largestFileName: files.slice(-1)[0][1],
        largestFile: files.slice(-1)[0][0]})}})})

logger.js

import avgLines from './index'

const args = process.argv.slice(/node/.test(process.argv[0]) ? 2 : 1).map(v => `'${v}'`).join(' ')
const log = str => {
  let bold = false
  const newStr = str.split('**').reduce((pv, v) => (bold = !bold, `${pv}\x1b[${bold ? 1 : 0}m${v}`))
  console.log(newStr)}

export default avgLines(args).then(
  ({numFiles, numLines, avgLines: avgLines$, linesInequality, largestFileName, largestFile}) => (
    log(`Checked **${numFiles}** files (**${numLines}** LOC):`),
    log(`  **80%** of files contain > **${avgLines$}** LOC`),
    log(`  **20%** of files contain **${(linesInequality * 100).toFixed(2)}%** of the total LOC`),
    log(`  The largest file (**${largestFileName}**) contains **${largestFile}** LOC`))).
  catch(::console.error)
\$\endgroup\$
2
  • \$\begingroup\$ So is the input a directory, and the output the paragraph at the beginning of your post? If there are more features, can you describe them at the top of your post? It's hard to give feedback without knowing exactly what the code is trying to accomplish... \$\endgroup\$
    – Jonah
    Mar 4, 2016 at 3:35
  • \$\begingroup\$ @Jonah Clarified question, the input arguments are the same as for find (bash). The output is the paragraph at the top. \$\endgroup\$
    – Ashton Six
    Mar 27, 2016 at 15:51

1 Answer 1

1
\$\begingroup\$

Ok, first, you've gone overboard. :D

You have a lot of regular expressions, but on first glance, I don't even know what they're for. What are they matching? It's best if you name them by putting them in const.

I see you have some personal touch on the code, like the ending parens being on the very last line. Makes the code look like CoffeeScript at first look. Anyways, code is for humans. Put some whitespace, break code into logically grouped lines, add some comments why you do things this way. One cannot really comprehend without tracing through (which is a sign of bad things to come).

Also, you're running this through Node, size doesn't matter there. If you run JS on a browser, there's what they call a minifier.

Also, variable names. Some make sense, some don't. l, v1, v0, pv, i80, they don't really tell me what they are. One terrible mistake devs often do is they assume the reader knows the context. Sure, pv means "previous value", but I only knew that because I know how reduce works. For someone that doesn't, that's a different story.

80% and 20% are hardcoded.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for taking the time to answer! I mostly optimize for fitting each module into a 100 * 45 character box to avoid scrolling. In more complex code I'll have ~[30% imports / 40% comments / 30% code] & use more intermediate variables which breaks up the structure. \$\endgroup\$
    – Ashton Six
    Feb 3, 2016 at 10:40

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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