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)
1 Answer 1
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.
-
\$\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– Ashton Six2016年02月03日 10:40:25 +00:00Commented Feb 3, 2016 at 10:40
find
(bash). The output is the paragraph at the top. \$\endgroup\$