Effective Code Reviews

Code reviews are a great way to improve code quality, catch bugs, and keep your whole team in sync with the latest changes. My favorite tool for doing a code review is still Github’s Pull Request feature, because of the easily-accessible diffs, inline comments, and commit-level and PR-level views of the file changes.

So, how do we make the code review process effective? I offer the following tips:

  • Actually review PR’s; telling an author to “just ship it” is detrimental.
  • Senior developers want you to review their code; don’t be scared.
  • Close PR’s that don’t have specs
  • If spec coverage is sparse, discuss it with the author (nicely).
  • Spec descriptions should accurately reflect the behavior of the module/method they describe.
  • Ask questions instead of making statements; the author likely put more thought into the code than you are aware of, and they’ve already solved the problem once.
  • Look for design problems:
    • Does this change solve a real problem? is it worth solving?
    • Given the constraints, is the overall approach correct and reasonable?
  • Look for code smells and suggest refactorings.
  • Ask for help with reviewing code that is outside your area of expertise.

ECMAScript 6 - Default, Rest, and Spread Arguments

Ever wanted a default value for an arugment? Or the ability to work with variable numbers of arguments more easily? Ever written code like this?

1
2
3
4
function(foo, bar) {
  bar = bar || 'default value';
  var theRest = [].slice.call(arguments, 2);
}

Perhaps you’re familiar with CoffeeScript’s splat arguments (...); or, if you’ve ever used Ruby, you’ll be familiar with the Ruby splat operator.

Maybe you’ve needed to use an array as an argument list? Ever done this?

1
2
var args = [1,2,3];
doSomethingWithArgs.apply(null, args);

The days of using [].slice.call and fn.apply(null, args) are over! Finally, JavaScript is introducing a splat-like operator! Say hello to default, rest, and spread arguments!

ECMAScript 6 - Arrow Functions

One of my favorite proposals for ECMAScript 6 is the proposal to add a convenient labmda syntax: Arrow Functions.

Personally, I get sick of typing function (){} all the time (ok, I don’t usually type it, my editor takes care of it).

Say I want to do some functional stuff (silly example):

1
2
3
4
5
var numbers = [0,1,2,3,4,5,6,7,8,9];
// map a list of even/odd pairs, multiplied
numbers
  .filter( (x) => { return x % 2 === 0;} )
  .map( (x, i) => { return x * numbers[i+1]; } );

Yay! Much more convenient. But, arrow functions are not exactly like functions. How, you ask?