Writing Readable Code

Line length

We have giant screens now!

Yes, but...

• You could fit multiple files
• You could be on a laptop screen
• Your teammate could be on a laptop screen
• Long lines are just hard to read!
  Typography: 45-90 characters on a line

Long line

this.document.deserialize(loadedPlugins, docData['state'], changesCallback, docData['action_history_length'], docData['id'], docData['next_connection_id'], docData['creator_id'], function() {...});
                        

Long line (fixed)

this.document.deserialize(
    loadedPlugins,
    docData['state'],
    changesCallback,
    docData['action_history_length'],
    docData['id'],
    docData['next_connection_id'],
    docData['creator_id'],
    function() {...}
});
                        

Comments love smelly code

What does this do?

container.on('scroll', goog.functions.throttle(function(e) {
    var b = this.body;
    if(b.scrollHeight - (b.scrollTop + b.clientHeight) < 150) {
        this.loadNextPage();
    }
}, 20, this))
                        

What does this do?

// infinite scrolling
container.on('scroll', goog.functions.throttle(function(e) {
    var b = this.body;
    if(b.scrollHeight - (b.scrollTop + b.clientHeight) < 150) {
        this.loadNextPage();
    }
}, 20, this))
                        

Comment obsolete

this.infiniteScrollHandler = goog.functions.throttle(function(e) {
    var b = this.body;
    if(b.scrollHeight - (b.scrollTop + b.clientHeight) < 150) {
        this.loadNextPage();
    }
}, 20, this);

container.on('scroll', this.infiniteScrollHandler);
                        

Does the comment add knowledge?

//focus on email field
$('#email').focus();
                        

Have to do something weird

/* can't group selectors with placeholders for different browsers, won't work :( */
::-webkit-input-placeholder {
    color: grey;
}
::-moz-placeholder {
    color: grey;
}
:-ms-input-placeholder {
    color: grey;
}
::placeholder {
    color: grey;
}
                        

Comments → log statements

val (domainPolicyOpt, domain) = Registration.getDomainPolicy(email)
domainPolicyOpt.flatMap { domainPolicy =>
  // domain policy found - add new user to existing account
  services.user.getDomainAccountId(domain).map { teamAccountId =>
    ...
  }
}
                        

Comments → log statements

val (domainPolicyOpt, domain) = Registration.getDomainPolicy(email)
domainPolicyOpt.flatMap { domainPolicy =>
  logger.debug(s"Domain policy found for $domain. Adding user to existing account")
  services.user.getDomainAccountId(domain).map { teamAccountId =>
    ...
  }
}
                        

Avoiding ambiguity

What does this do?

teamUserProvisioner.processLogin(
    newUser,
    "free",
    productAccount
)
                        

Generic verbs

• check
• validate
• make
• get/set
• process
• start

Some alternatives

Can’t come up with a verb?

Maybe your method is doing too much!

Consistency matters!

All code in any code-base should look like a single person typed it, no matter how many people contributed.

Naming booleans

What does this do?

function showOneDriveImport() {...}

                        

function showOneDriveImport() {
    var maybeFeature = this.optfeatures.getFeature('oneDriveImageImport');
    return !!maybeFeature && maybeFeature.isEnabled();
}
                        

Communicate truth

It should be obvious that the value is a boolean

• shouldShowOneDriveImport
• isEmpty
• canCreateDocuments
• hasLicense

The boolean trap

What does this do?

page.getSVG(dpi, true, false)

                        

Deobfuscate boolean parameters

page.getSVG({
    imageDpi: dpi,
    includePageBackground: true,
    compress: false
})
                        

Deobfuscate boolean parameters

page.getSVG(
    /* imageDpi */              dpi,
    /* includePageBackground */ true,
    /* compress */              false
)