0 1


talk on code quality for javascript

On Github timruffles / javascript-code-quality

Code quality is meaningless

@timruffles @sidekicksrc

WTF is code quality?

Not application quality

  • e.g Slow
  • Doesn't work
  • Poor design
  • Basically: IE6

Everything the user can't see

  • and doesn't want to
  • like 'plumbing quality': just want the toilet to flush
  • can have good app with bad code, and reverse

Who does it anger?

  • app quality: users
  • code quality: programmers

Code quality is for programmers

WTF is good code?

WTF is a good film?

How can a film be 4/5 stars for everyone?

Film quality...

...depends on the person

  • I'm not going to rate 'Fast n Furious 6' 5/5
  • Might have done when I was 14

...and doesn't

  • e.g out of focus, mistakes in plot
  • well made, but of a genre I don't like
  • wish film critics realised this

What 'quality' means depends on us

Unfortunately... programmers are faddish and emotional people

Quality is the most subjective bit of coding

Idiomatic Haskell

go a@(Internal al ar abyte abits) ak b@(Internal bl br bbyte bbits) bk
  | (dbyte, dbits) < min (abyte, abits) (bbyte, bbits) = fork a ak b bk
  | otherwise =
       case compare (abyte, abits) (bbyte, bbits) of
         LT -> splitA a ak b bk
         GT -> splitB a ak b bk
         EQ -> link a (go al ak bl bk) (go ar (minKey ar) br (minKey br))

Obvious hey?

Or 'good OOP'

@garybernhardt For example: Looking at this code is a great way to demonstrate how terrible it is to treat AR as a DAO:

— DHH (@dhh) December 17, 2012

Can we leave taste aside...?

  • OOP or FP or procedural

...what's left?

  • "This does the same things 3 ways"
  • "I can change anything!"
  • "This looks like Java!"
  • "WTF?! Why did that happen?!"

So code quality can be given a meaning

We can agree on the adjectives for good code

...or bad

Aspects of quality code

  • Concise
  • Simple
  • Readable
  • Extensible
  • Testable


A sentence should contain no unnecessary words, a paragraph no unnecessary sentences.

William Strunk Jr. in Elements of Style

Flabby code

function email(string) {
  if(emailRe.test(string)) {
    return true;
  } else {
    return false;

Don't repeat yourself


Programming is pretty much DRY applied to life

Problems of non-DRY code

  • fragile: change in n-1 places = bug
  • maintainance overhead

How to track?

  • flay (hack it to do JS with esprima), flay-js?
  • structural duplication detection

Flay in Sidekick

Single point of truth


Related to DRY

Each fact about the system captured once


  • How old does a user need to be?
  • Scatter in view, model etc: bad
  • Capture the knowledge in one place
  • More complex things: capture in one function
$("form").submit(function(el) {
  if($(el).find("[name=age]").val() > 18) {



Actually, nobody tries to write complex code

So this is a tricky word

We should probably abandon 'simple' and 'complex'


from Latin simplus. Originally referred to a medicine made from one constituent

  • vs complex, something made from many parts
  • Rich Hickey's definition
  • 'doing one thing well'


Flavours of simple

  • low number of concepts
  • low detail
  • few responsibilies
  • low difficulty
  • few dependencies/interactions

Low number of concepts

  • no OOP, FP, advanced language features
  • ORM vs SQL
function main() {
  // ... 5000 lines

Low detail

  • attempt to abstract everything
function Account() {
Account.prototype.transfer = function(anotherAccount) {
  // very little work per method
  return new AccountTransfer(this,anotherAccount).run();
<h2>{{ }} is HTML made awesome!</h2>

Few responsibilies

Low difficulty

Few dependencies

function sprintf(format,...args) {
  // completely depends on input
function UserView() {
  // depends on everything
  this.el = $("#user");
  this.user = $.get("/users/" +;

Can we measure these?


  • low number of concepts
  • low detail
  • few responsibilies
  • low difficulty
  • few dependencies/interactions

Low difficulty

  • code that doesn't have much to do is easy to understand
  • global vs local

Global - low difficulty

  • "write a static site generator with markdown"
  • simple code - straight transformation

Global - high difficulty

  • "integrate the UK's medical records"
  • OMG complex, operations, integration, special cases


  • 500 LOC functions: doing a lot
  • 10 LOC functions: each function easy to understand
  • 15 variables with many branches
  • no branches, simple transform

Measuring global difficulty

  • How frightened are you of your code/deploy/monitoring?

Measuring local difficulty

  • how many paths?
if(x) {
} else {
if(y) {
} else {

Branching factor

  • 4 paths: tt, tf, ft, ff
  • Cyclomatic complexity: 3
if(x) {
} else {
if(y) {
} else {

Variable count

$digest: function() {
  var watch, value, last,
      asyncQueue = this.$$asyncQueue,
      postDigestQueue = this.$$postDigestQueue,
      dirty, ttl = TTL,
      next, current, target = this,
      watchLog = [],
      logIdx, logMsg, asyncTask;


  // Insanity Warning: scope depth-first traversal
  // yes, this code is a bit crazy, but it works and we have tests to prove it!
  // this piece should be kept in sync with the traversal in $broadcast
  if (!(next = (current.$$childHead || (current !== target && current.$$nextSibling)))) {
    while(current !== target && !(next = current.$$nextSibling)) {
      current = current.$parent;
} while ((current = next));

How many things can you keep in your head at once?

Metrics for difficulty

  • logical lines of code
  • cylomatic complexity: branching
  • halstead complexity: operands and operations
  • ComplexityReport offers both, JSHint has CC cutoff

Measuring dependencies

The more interconnected the code, the harder to change

Worst type: temporaral

  • e.g app.js touches global variable user, user_view.js relies on it
  • very hard to see

DOM = global

  • $(".class") is a global
  • always moduarlise DOM, passing in element to components


  • doesn't just mean having a module system
  • unless the code is largely independent, it's not modular
// global variable - regardless of the 'module' in there
require("models/user.js").userCount += 1;
// exactly the same
(global || window).require("models/user.js").userCount += 1;


  • ComplexityReport - entanglement metric
  • detects nth level dependencies (e.g A -> B -> C)



Means specific to the language

e.g Javascript

;(function(undefined) {

  window.something = something;

  function something(n,opts) {
    var user = opts && opts.user;
    var n = run == null ? 0 : n;
    var names = [],2);
    for(var p in x) {
      if(x.hasOwnProperty(p)) // ...

...and not bringing other languages with you

The obvious tool to use is...


  • uses esprima parser
  • much nicer code-quality than JSHint's Pratt parser


  • featureful


  • would be nice to have library/framework linters

Good naming

A recipe

  • longer the bigger the context
  • no single letters beyond i, k, and v
  • avoid abbreviation, hard to be consistent
  • no types
  • replace trivial comments with variables or functions

Good comments

Comment when

  • intention not clear from code
  • a variable or function name not enough
  • there's a surprise

Comments are not documentation

Documentation is...

  • example code (tests are good, cf. doctest.js)
  • explanations of the system at high-level


Happens a lot less than you think

  • overpromised
  • how often is what you're writing generic enough to be reused?

Is not good per se

If you’re willing to restrict the flexibility of your approach, you can almost always do something better

John Carmack

More generic, more reusable

  • without effort
  • e.g lodash
  • FP achieves 'reuse in the small', OOP hasn't (too specific)


What prevents change?


Large APIs

  • the more methods a module has, the more dependencies it'll grow

Preventing ad-hoc reuse

  • Javascript has VERY primitive privacy
var Public = (function() {
  var cantSeeMe = //;
  function Public() {
  function privateA() {
  return Public;

Allowing ad-hoc modification

  • the C/python way: _somethingPrivate
  • good for internal code, more risky for external
var Public = (function() {
  function Public() {
  Public._canSeeMe = //;
  Public._privateA = function () {
  return Public;


Test 0

Can you test your code?

Sometimes, easily

function sprintf(str) {
  var params = [],1)
  var types = {
    "%s": function(x) { return x + "" }
  var matches = 0
  return str.replace(/(%[is])/g,function(match) {
    return types[match](params[matches++])

it("formats correctly",function() {

Often this simple?

Get real

'generateResetToken': function (req, res) {
    var email =;

    api.users.generateResetToken(email).then(function (token) {
        var siteLink = '<a href="' + config().url + '">' + config().url + '</a>',
            resetUrl = config().url.replace(/\/$/, '') +  '/ghost/reset/' + token + '/',
            resetLink = '<a href="' + resetUrl + '">' + resetUrl + '</a>',
            message = {
                to: email,
                subject: 'Reset Password',
                html: '<p><strong>Hello!</strong></p>' +
                      '<p>A request has been made to reset the password on the site ' + siteLink + '.</p>' +
                      '<p>Please follow the link below to reset your password:<br><br>' + resetLink + '</p>' +

        return mailer.send(message);
    }).then(function success() {
        var notification = {
            type: 'success',
            message: 'Check your email for further instructions',
            status: 'passive',
            id: 'successresetpw'

        return api.notifications.add(notification).then(function () {
            res.json(200, {redirect: config.paths().webroot + '/ghost/signin/'});

    }, function failure(error) {
        // TODO: This is kind of sketchy, depends on magic string error.message from Bookshelf.
        // TODO: It's debatable whether we want to just tell the user we sent the email in this case or not, we are giving away sensitive info here.
        if (error && error.message === 'EmptyResponse') {
            error.message = "Invalid email address";

        res.json(401, {error: error.message});

Correlates with rest of quality

Anti-testable code is...

  • coupled
  • closed to modification
  • complex (doing > 1 thing)

Measuring tests

  • istanbul: measures % of LOC run by tests

Putting it all togther

  • CI server running tests and coverage tools
  • Also run ComplexityReport, Flay, JSHint
  • JSHint fast enough for a pre-commit hook, often good as 'poor man's compiler'

All the tools

  • JSHint/ESLint/JSLint
  • ComplexityReport
  • Flay
  • Istanbul

Now you've measured the quality, what now?

Measured == managed?


That can be bad

Use responsibly

Weigh the airplane

Gaming is easy


∞-junior, idiot-savant

Conversation starter

Wall o’metrics

Stay proud of your team’s code

Tools are good source of information

You’re the expert

Any questions?


Creative commons work