JS Coding Convention

Improvements? Suggestions? email lif.zone.main+dna@gmail.com

Coding conventions used by other companies: AirBNB | google | crockford | wordpress | drupal | weflex

Overview: consistent & minimal

Be consistent

If there is no specific rule in this document - be consistent with existing codebase (use rgrep).
If you're editing code, take a few minutes to look at the code around you and determine its style.
If it prints error messages starting with a capital letter, you should too. If it put spaces around complex expressions but not around simple expressions, your complex expressions should also have spaces around them.
The point of having style guidelines is to have a common vocabulary of coding, so people can concentrate on what you're saying rather than on how you're saying it.
If code you add to a file looks drastically different from the existing code around it, it throws readers out of their rhythm when they go to read it.
Avoid this.

Minimalistic and Condense

Wherever there is no specific rule, always prefer minimalism.
Less tokens, higher condensity: get as much code as possible visible within the screen, so less scrolling is needed.
Sparse, elaborate, defensive code is not appreciated.
An example of absurd over-engineering. Although it's for Java, but the general idea holds for JS.

Tools

We write code by hand, line-by-line. We do have tools to assist us locating conventions mistakes zlint -c, but they are just a 'helpers'.
The tools match our conventions only 95%, so its still our personal responsibility to manually make sure line-by-line the code we write matches the conventions 100%, whether the tool finds the mistakes or not.

Text file layout

Tab size is 8 spaces.
Shift width (indentation) is 4 spaces.
Column width 79 char, so it fits well with 80 col terminal.
Indentation is always one shift-width(4):

open_msg_box("closing file %s on server ", file_name); open_msg_box("closing file %s on server ", file_name);

Formatting and naming

Code blocks and statements

  • if for while don't require a code block for 0 or 1 statements: open { on next line. only use a block for multiline blocks.
  • try catch function require a code block in all cases: open { on the same line.

if/for/while

if/for/while block

if for while open and close braces of a section should be on the same level.

if (pkt) { pkt.close(); pkt.uninit(); } if (pkt) { pkt.close(); pkt.uninit(); }

if for while statement that takes more than one line should always have braces
same thing goes when the then/loop part is more than one line

if (slot.dw_bus==pci_scan.card_slot[ret].dw_bus && slot.dw_slot==pci_scan.card_slot[ret].dw_slot && slot.dw_function==pci_scan.card_slot[ret].dw_function) { break; } if (x==y) { my_func(param1, param2, param3, param4, param5, param6, param7); } for (ret=0; ret<pci_scan.dw_cards; ret++) if (slot.dw_bus==pci_scan.card_slot[ret].dw_bus && slot.dw_slot==pci_scan.card_slot[ret].dw_slot && slot.dw_function==pci_scan.card_slot[ret].dw_function) break; for (ret=0; ret<pci_scan.dw_cards; ret++) { if (slot.dw_bus==pci_scan.card_slot[ret].dw_bus && slot.dw_slot==pci_scan.card_slot[ret].dw_slot && slot.dw_function==pci_scan.card_slot[ret].dw_function) { break; } }

if/for/while without a statement

while(pop_first(list)) ; while (pop_first(list)); for(i=0; i<10; i++) ; for (i=0; i<10; i++); if (a>b+10) ; else if (a>b+5) do_x(); else if (a>b+2) ; else do_y(); if (a>b+10); else if (a>b+5) do_x(); else if (a>b+2); else do_y();

Then

'then' part of if statement should be in a separate line.

if (close_file) fclose(fp); if (close_file) fclose(fp);

else if

else if statements should be on the same level at the starting if reason: this is similar to switch statement

if (argv[1]==="--help") print_usage(); else if (argv[1]==="--run") { run_application(); print_results(); } else print_error(); if (argv[1]==="--help") print_usage(); else if (argv[1]==="--run") { run_application(); print_results(); } else print_error();

Functions

Tiny functions: same line.
One-liner: body in second line.
Two lines and above: a proper block. if anon function: block open in same line.

var tiny = function(){code;}; var tiny = function() { code; }; var tiny = function(){ code; }; var tiny = function() { code; }; var tiny = function(){ code; }; var tiny = function(){ code; }; function tiny(){code;}; function tiny() { code; }; function tiny() { code; } function tiny(){ code; } function tiny(){ code; } function tiny(){ code; } function long(args){ code; code; } function long(args) { code; code; } function long(args){ code; code; } function long_args(a1, a2, a3, a4){ code; code; } function long_args(a1, a2, a3, a4) { code; code; }

Inline functions

Function definition that fits the line:

let x = parse_args(...args, function(line){ let escaped = E.escape(line); ... });

Function definition that does not fit the line:

let x = parse_args(a, b, c, d, e, function(line, err) { let escaped = E.escape(line); ... }); let x = parse_args(a, b, c, d, e, function(line, err) { let escaped = E.escape(line); ... }); let x = parse_args(a, b, c, d, e, function(line, err){ let escaped = E.escape(line); ... }); let x = parse_args(a, b, c, d, e, function(line, err) { let escaped = E.escape(line); ... });

Functions spacing

No space between function name and opening parenthesis
No space between opening parenthesis and first parameter
One space after comma:

printf ("hello %s ", "world"); printf( "hello world " ); printf("hello world ","world"); printf("hello %s ", "world");

Breaking a long line

When breaking up a long line, it should continue with one shift-width for indentation

if (line_length>1 && (screen.sz.vertical<buffer.sz.vertical || explicit_lines)) { console.log("this is a test section that will show how to handle "+ "long lines, such as this one which is 2 lines long"); } if (line_length>1 && (screen.sz.vertical<buffer.sz.vertical || explicit_lines)) { console.log('this is a test section that will show how to handle '+ 'long lines, such as this one which is 2 lines long'); }

switch statements

switch statements should have case on the same level, no space before :.

switch (key) { case KEY_UP : key = UP; break; case KEY_DOWN : key = DN; break; default : key = NONE; } switch (key) { case KEY_UP: key = UP; break; case KEY_DOWN: key = DN; break; default: key = NONE; } switch (key) { case KEY_UP: key = UP; break; case KEY_DOWN: key = DN; break; default: key = NONE; }

1-liner case: with single statement plus break, or a return statement.

switch (key) { case KEY_UP: line--; break; case KEY_DOWN: for (x=line; x; x--) line++; break; case KEY_ESC: return; default: bubble = true; } switch (key) { case KEY_UP: line--; break; case KEY_DOWN: for (x=line; x; x--) line++; break; case KEY_ESC: return; default: bubble = true; }

Never break after return

switch (key) { case KEY_UP: line--; break; case KEY_ESC: return; break; } switch (key) { case KEY_UP: line--; break; case KEY_ESC: return; }

Never break at end of default

switch (key) { case KEY_UP: line--; break; default: line++; break; } switch (key) { case KEY_UP: line--; break; default: line++; }

Reserved words

One space after reserved words before the opening parenthesis, except for function and catch (which is function like):

if(close_file) if (close_file) for(i=0; !is_last(i); i++); for (i=0; !is_last(i); i++); try{ code; } catch (e) { code; } try { code; } catch(e){ code; } var t = function (api) { api('test'); }; var t = function(api){ api('test'); };

try-catch

Multiline try: close block on catch line (like do-while):

try { short_code; } catch(e){ code; } try { longer_code; } catch(e){ code; } try { and_even; longer_code; } catch(e){ code; }

Catch variable should be named 'e'

try { res = yield etask.nfn_apply(zmongo.collection, '.save', [obj]); } catch(error){ handle_error(zmongo, 'save', error, obj); } try { res = yield etask.nfn_apply(zmongo.collection, '.save', [obj]); } catch(e){ handle_error(zmongo, 'save', e, obj); }

Operator spacing

both sides should be equal: either space before and after, or no spaces at all.

Trivial > >= < <= == != expressions

Trivial > >= < <= == != expressions should not have spaces around them: a_var.

if (x> 5) if (x >5) if (x > 5) if (x>5)

Nearly-trivial expressions can be with or without spaces: a[3], f(x), x.y.z, a.b.c(x, y, z).

if (f(x, y) > g(y, z)) if (f(x, y)>g(y, z)) if (x.y == 5) if (x.y==5) if (x.y>=5) if (x.y >= 5)

We consider simple short arithmetics also as nearly-trivial expressions: x+1, 2*x.

if (x+1>5) if (x+1 > 5)

if one side is not trivial, then must have spaces.

if (a&&a.b) if (a && a.b) if (!a&&a.b) if (!a && a.b)

if one side is long, then prefer to have spaces.

if (a&&Array.isArray(a)) if (a && Array.isArray(a))

Assignments spaces

Spaces around assignments = += -= *= /= &= |= are not mandatory in for() loops.

a=b; d+=x; a = b; d += x; for (i=0; i<10; i+=4); for (i = 0; i<10; i += 4);

Unary operators

Don't put a space after ++ -- !, and other unary operators. increment after the value, not before.

i --; ++j; i--; j++; if (! i) if (!i) var speed_int = + speed_str; var speed_int = +speed_str;

Multiple +

Multiple + operators are trivial expressions, and thus we prefer not to put spaces around them (such as string concatenation).

msg = 'hello ' + name + '!'; msg = 'hello '+name+'!'; msg = 'hello '+get_my_name()+'!'; msg = 'hello ' + get_my_name()+'!';

Conditionals ? :

? : should have spaces all around them.

var msg = login_ok?'Welcome':'Please login'; var msg = login_ok ? 'Welcome' : 'Please login';

Minimalism

Do not use unneeded parentheses in expressions:

if ((a!=b) || (c!=d)) ... if (a!=b || c!=d) ...

Avoid empty lines

Never in any case have two empty lines together.
A single empty line is allowed in global scope and between functions, but use it sparsely.

Separate line

Should not separate with more than one blank line between sections, functions etc.

function init(){ ...; }; function end(){ ...; }; function init(){ ...; }; function end(){ ...; }; function init(){ ...; }; function end(){ ...; };

Inside functions, should never have an empty line. Use comments to separate.

function setup_conn(orig){ var conn = net.connect(dst); conn.pipe(orig); conn.set_speed('fast'); if (conn.rx>MAX_SPEED) conn.set_speed('medium'); return conn; } function setup_conn(orig){ var conn = net.connect(dst); conn.pipe(orig); // setup speed conn.set_speed('fast'); if (conn.rx>MAX_SPEED) conn.set_speed('medium'); return conn; } function setup_conn(orig){ var conn = net.connect(dst); conn.pipe(orig); conn.set_speed('fast'); if (conn.rx>MAX_SPEED) conn.set_speed('medium'); return conn; }

Arrow function short syntax

When possible, use arrow function short syntax

let t = data=>{ return do_something(data); } let t = data=>do_something(data);

return statements

Minimal return statement

return statements should not have parentheses and should not have a space after them:

return (0); return 0; return ; return;

return undefined

For return undefined, just do return, undefined is the default of JS.

If a function returns undefined at exit, you don't need to call return.

function x(x){ if (!x) return undefined; ...; if (x_is_valid(x)) return x+5; ...; } function x(x){ if (!x) return; ...; if (x_is_valid(x)) return x+5; ...; }

return an assignment

You may return an assignment in order to merge assignment and return statments. Do not add unneeded brackets

return (a = b); return a = b;

Cast to void

You may 'cast' to void in order to merge an action and return undefined into a single line.

return void sock.close(); if (ret) return void (cache[id] = ret); if (clock) clock = void clock.restore();

Compare to 0

When 0 stands as non valid or empty option, avoid comparing to 0

if (total==0) return 'NA'; if (!total) return 'NA';

Long strings

Breaking up long strings: no space around +, and prefer + at beginning of next line.

msg = 'this ' + 'is a long string.'; msg = 'this ' + 'is a long string.'; msg = 'this '+ 'is a long string.'; msg = 'this ' +'is a long string.'; msg = 'this ' +'is a long string.';

Space, if needed should be at the end of each line and not at the beginning of the next line

msg = 'this' +' is a long string'; msg = 'this ' +'is a long string';

File-level closures

File-level closures: do not indent the whole file. leave a space after the opening, and before the closing:

(function($, chrome, console){ code; code; })(jQuery, chrome, console); (function($, chrome, console){ code; code; })(jQuery, chrome, console);

JS file template

IE/Chrome/FF template

// LICENSE_CODE ZON 'use strict'; /*jslint browser:true*/

NodeJS application template

#!/usr/bin/env node // LICENSE_CODE ZON 'use strict'; /*jslint node:true*/ require('util/config.js');

NodeJS module template

// LICENSE_CODE ZON 'use strict'; /*jslint node:true*/ require('util/config.js');

require()

Local packages should include .js in the name. Global packages should be the package name only (NODE_PATH environment variable should be defined).

const express = require('/usr/lib/node_modules/express'); const express = require('express'); const get_rules = require('./get_rules'); const get_rules = require('./get_rules.js');

require should be called at the top of the file, not locally, so that unit-tests will surface all the problems.

function readconf(){ return require('fs').readFileSync('conf'); } // top of file... const fs = require('fs'); // later down in the file... function f(){ return fs.readFileSync('conf'); }

Constants and variable names of required modules should normally be the same, according to the module name.

const rules = require('./get_rules.js'); // in foo.js const actions = require('./get_rules.js'); // in bar.js const get_rules = require('./get_rules.js'); // in foo.js and bar.js

Use _ instead of - /

const node_getopt = require('node-getopt'); const svc_reconf = require('../svc/reconf.js');

Always prepend z to variable names for the following modules from pkg/util, and, when necessary, for other modules that clash with global packages.

const zconf = require('util/config.js'); const zconf = require('util/config_int.js'); const zescape = require('util/escape.js'); const zhttp = require('util/zhttp.js'); const zhttp = require('util/http.js'); const zurl = require('util/url.js'); const zutil = require('util/util.js'); const zos = require('util/os.js');

You may drop the node- prefix, or -js suffix.

const getopt = require('node-getopt'); const uglify = require('uglify-js');

Special modules with short names: jquery and underscore.

var $ = require('jquery'); var _ = require('underscore');

Comments

Prefer C++ comment over C comments.

/* close all files */ fclose(fp); // close all files fclose(fp); /* multi * line * comment */ // multi // line // comment

Comments aligned

Comments should be aligned as the code they comment, or one space after the end of the line.

/* * close all files */ fclose(fp); /* close all files */ fclose(fp); fclose(fp); /* close all files */

Long comments

Comments which occupy more than one line should adhere to the following guideline:

// // close all the files that were opened before the function was called // and send them to the output file // fclose(fp); // close all the files that were opened before the function was called // and send them to the output file fclose(fp);

Multiline comments

Multiline comments should always be on their own, not continue on existing statements. If longer, put the comment at the line above.

var isa_pnp_scan = { search_id: 'a-test', // if search_id.vendor[0]==0 - scan all vendor // IDs // if searchId.dwSerial==0 - scan all serial // numbers cards: 2, // number of cards found card: [card1, card2], // cards found }; var isa_pnp_scan = { search_id: 'a-test', // if search_id.vendor[0]==0 - scan // all vendor IDs // if searchId.dwSerial==0 - scan all // serial numbers cards: 2, // number of cards found card: [card1, card2]; // cards found }; var isa_pnp_scan = { // if search_id.vendor[0]==0 - scan all vendor IDs // if searchId.dwSerial==0 - scan all serial numbers search_id: 's-test', cards: 2, // number of cards found card: [card1, card2], // cards found };

XXX - todo mark

Comments of XXX should be used for temporary code, that would be changed in the future: hack, quick workaround, non-elegant code, bug. They should be XXX [login]: COMMENT, multiple names should be separated with /.

XXX derry: ANDROID move to compat XXX derry: arik: move to compat XXX derry arik: move to compat XXX derry: move to compat XXX derry ANDROID: move to compat XXX derry/arik: move to compat

XXX comments are tasks, so they should always have someone assigned. Preferably yourself!

XXX: move to compat XXX some-other-developer: move to compat XXX derry: move to compat

Using XXX comments can be used, when needed, to override any possible rules!

if (0) it('pause_disconnect', ()=>etask(function*(){ if (0) // XXX sergey: lots of sporadic failues it('pause_disconnect', ()=>etask(function*(){ // XXX derry: copied urgent gist as-is to hack around a bug from angular // http://github.com/.... // will fix by 8-Nov-2016

Loops

for vs while

If a while loop has an init and/or next expression, then use a for loop:

i = 0; while (i<10) { ...; i++; } for (i = 0; i<10; i++) ...;

If a for loop has only a condition, then use while:

for (; have_more();) ...; while (have_more()) ...;

If no init/condition/next, then any option is ok:

for (;;) do_endless_work(); while (1) do_endless_work();

for loops

In for loops, when there is a function that gets the next element, it should be done once (inside the step condition):
assign inside a statement:

for (i = 0, result = get_char(); result==' '; result = get_char(), i++) handle_result(result); for (i = 0; (result = get_char())==' '; i++) handle_result(result);

do-while

do-while: long do block should be closed on while line:

do add_item(); while (have_items); do add_item(); while (have_items); do { add_item(); another_action(); } while (have_items);

Spacing

Put one space before and after an assignment.

var a=0; x= x+1; bits|=BIT5; var a = 0; x = x+1; bits |= BIT5;

If it is in a for loop, you can skip the spaces. if you skip one of the spaces, skip both:

for (i= 0; i; i--) printf("%d", i); for (i=0; i; i--) printf("%d", i); for (i = 0; i; i--) printf("%d", i);

Don't put a space before a statement separator, put one after it:

for (i=0 ;i ;i--, j*=2) ; for (i=0; i; i--, j*=2); for (i = 0; i; i--, j *= 2);

Check fail in same line of call

Prefer to check for failures in same line of calling the function.

i = str.indexOf(" "); if (i<0) return; if ((i = str.indexOf(" "))<0) return; m = str.match("EOF"); if (!m) return; if (!(m = str.match("EOF"))) return;

Object notation

var node = { name: 'server', port: 42, status: 'updated', setup_time: 10*1000, }; var node = { name: 'server', port: 42, status: 'updated', setup_time: 10*1000, };

Multiline objects: should have a comma after last property.

var node = { name: 'server', status: 'updated', setup_time: 10*1000 }; var node = { name: 'server', status: 'updated', setup_time: 10*1000, };

Short objects: should be in a single line if they are very short.

var node = {name: 'server', port: 42,}; var node = { name: 'server', port: 42, }; var node = {name: 'server', port: 42};

Object contains short object: should be in a single line.

var node = {info: {name: 'server', port: 42}, }; var node = { info: {name: 'server', port: 42}, }; var node = {info: {name: 'server', port: 42}};

Objects spacing:

  • before , and : signs, there should not be a space, while after those signs, space is required.
  • after { and before } signs, there should not be a space.
var node = {name: 'server', port: 42}; var node = {name: 'server' , port: 42}; var node = {name : 'server', port : 42}; var node = { name: 'server', port: 42 };

Casting

Convert to number: +val
Convert to signed 32bit int: val|0
Convert to unsigned 32bit int: val>>>0
Convert to Boolean: !!val
Convert Boolean to 0/1: +bool_val or +!!any_val

String literals: prefer 'single quotes' over "double quotes"

"I am the walrus." 'I am the walrus.' "I'm the walrus."

exports

Use variable E as alias to exports.

exports.foo = function(){ ... }; const E = exports; E.foo = function(){ ... };

Export functions

Only export functions which are used outside of the module, keep everything else local.

E.internal_helper = function(){ ... }; ... E.internal_helper(); function internal_helper(){ ... } ... internal_helper();

Unit-test exports

Use variable E.t for exports only used in tests, give the function name to use inside module.

// in foo.js E.internal_function = function(){ ... }; ... E.internal_function(); // in test.js foo.internal_function(); // in foo.js const E = exports; ... function internal_function(){ ... } ... internal_function(); ... E.t = {internal_function: internal_function, ...}; // in test.js foo.t.internal_function();

Continuation .method()

Continuation .method() or +'str' on next line can be same indentation as parent line. Same goes for single var definition, and return.

$('<h1>', $('<div>') .append('<span>')); $('<h1>', $('<div>') .append('<span>')); $('<div>') .append('<span>'); $('<div>') .append('<span>'); elm = $('<span>') .append('<span>'); var elm = $('<span>') .append('<span>'); var e1 = $('<div>'), e2 = $('<span>') .append('<span>'); var e1 = $('<div>'); var e2 = $('<span>') .append('<span>'); return $('<div>') .append('<span>'); return $('<h2>', $('<div>') .append('<span>')); return $('<h2>', $('<div>') .append('<span>')); var s = '<div>' +'<span>'; s = x ? '<child>' : '<nothing>' +'<span>'; s = '<div>' +'<span>'; return '<div>' +'<span>';

. of .method()

. of .method() MUST be the first character on a method call continuation line.

set_user(db.open('users'). get$(user)); set_user(db.open('users') .get$(user)); $('<h1>', $('<div>'). append('<span>')); $('<h1>', $('<div>') .append('<span>'));

Variable declarations

var declarations longer than one line must have their own var.

var a = 'test', b = f('another', 'test'), c = 'yet another'; var a = 'test'; var b = f('another', 'test'); var c = 'yet another'; var a = 'test'; var b = f('another', 'test'), c = 'yet another'; var a = 'test', b = f('another', 'test'), c = 'yet another'; var a = 'test', b = f('another', 'test'); var c = 'yet another'; var a = 'test'; var b = f('another', 'test'); var c = 'yet another';

Comparing variables

When comparing to undefined use === and !== On any other case, use == and !=

if (a==='OK') if (typeof a==='undefined') if (a=='OK') if (a===undefined)

Prefer .bind() over this

When assigning functions that depend on this, use bind().

var log = console.log; log('a'); // TypeError: Illegal invocation var log = console.log.bind(console); log('a');

Shorten using locality

Use locality to shorten names, relying on the contexts of the local code scope.

for (let opration_idx=0; operation_idx<operations.length; operation_idx++) ...; for (let i=0; i<operations.length; i++) ...; for (let allowed_customer in allowed_customers) ...; for (let c in allowed_customers) ...;

UNIX notation naming

Code will be in UNIX notation. UNIX names should not include the data type, rather the meaning of the information in the data.

var bIsCompleted; var iCount; var is_completed, data, vendor_id, count, buffer, new_name, name; function read_data_block(){} const MS_PER_SEC = 1000;

Positive naming

Default to using positive naming, rather than negative (no/not/disable...). This helps avoid double negation (not not).

if (!not_customer(customer)) disable_customer(customer, false); if (is_customer(customer)) enable_customer(customer, true); var no_rule = lookup_rule(rule)<0; if (!no_rule) rm_rule(); else add_rule(); var have_rule = lookup_rule(rule)>=0; if (have_rule) rm_rule(); else add_rule();

Simplicity and usability

Command line options, or option variable (opt), use common usage as default even if negative

zlxc --browser zlxc --no-browser opt = {exit_on_err: 1}; opt = {no_exit: 1};

Default value

Using implicit undefined as default value

let need_reconf = false; if (is_host(':servers')) need_reconf = true; let need_reconf = undefined if (is_host(':servers')) need_reconf = true; let need_reconf; if (is_host(':servers')) need_reconf = true;

Multi-signature functions

For functions that have multiple signatures or where there is an optional (not last) argument, you may optionally add the different possible signatures as a comment, in the nodejs signature documentation style.

// _apply(opt, func[, _this], args) // _apply(opt, object, method, args) E._apply = function(opt, func, _this, args){ ... };

Saving the value of this

When saving the value of this for use in lexically nested functions, use _this as the variable name.

var self = this; var _this = this; function on_end(opt){ var _this = this; return function on_end_cb(msg){ if (_this.socket) return 'socket'; }; }

Use __this and ___this... for deep code.

function on_end(opt){ var _this = this; return function(msg){ if (_this.socket) return 'socket'; var __this = this; setTimeout(function(){ __this.socket = undefined; }, 1000); }; }

Functions as class definition

When a function is a class definition, e.g. needs new in order to use it, it should start with capital letter

function etask(opt, states){ ... } function Etask(opt, states){ ... }

ES6

Arrow functions

No spaces around =>. Prefer to drop ()

e.on('play', () => { player.start(); started = 1; } e.on('play', ()=>{ player.start(); started = 1; } socket.on('connect', ()=> state = 'CONNECTED' ); socket.on('connect', ()=>state = 'CONNECTED'); docs.forEach(doc => add(doc)); docs.forEach(doc=>add(doc)); docs.forEach((doc, index)=>{ if (index) add(doc, index); });

Drop () around single argument.

docs.forEach((doc)=>add(doc)); docs.forEach(doc=>add(doc));

Prefer to drop {} around single short statement.

docs.forEach((doc)=>{ add(doc); }); docs.forEach(doc=>add(doc));

Preferred Methods

never use .indexOf() for arrays/strings when .includes() fits.

if (apps.indexOf(zserver_match[1])<0) apps.push(zserver_match[1]); if (!apps.includes(zserver_match[1])) apps.push(zserver_match[1]);

never user .indexOf() for strings when .startsWith() fits.

return !patch[0].indexOf(changed_file) || !patch[1].indexOf(changed_file); return patch[0].startsWith(changed_file) || patch[1].startsWith(changed_file);

Generators

No spaces around *.

etask(function* (){ ... }); etask(function*(){ ... }); etask(function * get_request(){ ... }); etask(function*get_request(){ ... });

Class definition

Class name start with capital leter

class SimpleView {} class simple_view {} class Simple_view {}

Add space between class name and {

class A{} class A {}

Etask methods

Indentation reducing is allowed, but class methods should be indented

class A { prop(){ return etask(function*(){ code; }); } } class A { prop(){ return etask(function*(){ code; }); } } class A { prop(){ let _this = this; return etask(function*(){ code; }); } } class A { prop(){ let _this = this; return etask(function*(){ code; }); } }

Programming technique

Generic code

Code should be written generically only if during the time you are writing it, it is called at least twice, if not more, and save code in the caller.
Generic code need a deeper unit-tests then regular code.

E.first_weekday_of_month = function(wd, d){ ... }; E.strftime = function(fmt, d, opt){ ... };

Early return

Avoid if() on 50% or more of a function.

let inited; E.init = ()=>{ if (!inited) { inited = true; register_app(); set_timers(); } }; let inited; E.init = ()=>{ if (inited) return; inited = true; register_app(); set_timers(); };

No defensive code

No function argument validation

function send_msg(client, msg, opt){ if (client===undefined || msg===undefined) throw new Error('send_msg: wrong params'); opt = opt||{}; msg = prepare_msg(msg); ... } function send_msg(client, msg, opt){ opt = opt||{}; msg = prepare_msg(msg); ... }

Assigning in a truth value (if or while)

Assigning truth value in if while for helps shorten and simplify code.

for (i = 0; get_result(i); i++) handle_result(get_result(i)); for (i = 0;; i++) { result = get_result(i); if (!result) break; handle_result(result); } for (i = 0; result = get_result(i); i++) handle_result(result); if (compute_num()) return compute_num(); if (x = compute_num()) return x; while (1) { i = input(); if (!i) break; handle_input(i); } while (i = input()) handle_input(i);

Temporary disabling a test

When temporary disabling test code that fail:
Do not indent the code of the disabled tests.

if (0) // XXX yoni: fails on BAT jtest_eq(...); if (0) // XXX derry: need fix for Ubuntu { jtest1(); jtest2(); } if (0) // XXX yoni: fails on BAT jtest_eq(...); if (0){ // XXX derry: need fix for Ubuntu jtest1(); jtest2(); }

If it is only one test (one statement), then don't use { } even if the statement is written in 2 lines:

if (0) // XXX: yoni: fails on BAT { jtest_run(xxx, yyy, zzz); } if (0) // XXX: yoni: fails on BAT jtest_run(xxx, yyy, zzz);

Open '{' on the same if() line:

// XXX: yoni: fails on BAT if (0) { jtest_run(xxx, yyy, zzz); jtest_run(xxx, yyy, zzz); } if (0){ // XXX: yoni: fails on BAT jtest_run(xxx, yyy, zzz); jtest_run(xxx, yyy, zzz); }

Performance

99% of the code is not performance critical. So always try to write shorter, simpler, more natural and modern code. If ES6 gives nicer simpler constructs - we use them.
But, in the rare 1% of the code that performs tight loops, we deviate from 'nice simple code', and write a little longer code, to avoid JS VM JIT in-efficiencies.
We normally check V8, and re-check check these issues periodically as newer versions of JS VM's come out.

for..of: 3x-20x slower

for (let p of patterns) add_pattern(p); for (let i=0; i<patterns.length; i++) add_pattern(patterns[i]);

Wrong performance assumptions

We list here commonly mistaken performance assumptions. They might have been correct in the past, but JS VMs get better and better - so these performance improvement assumptions are not longer correct.

Map is faster than Object

For keys that are plain positive numbers, Object may be faster due to Array optimizations in the VM. But for keys that are strings - Map is faster.

let cache = {}; cache[key] = value; let cache = new Map(); cache.set(key, value);

Deep Fix

When you (or someone else) find a coding mistake in your code:
BAD: just fixing that specific mistake you found
OK: fixing also all such mistakes you made in your own code
GOOD: fixing all such mistakes, in the whole codebase, using smart rgrep's to find them.
GREAT: if this is a common mistake people tend to repeat, then once a month repeat the search to find such NEW mistakes.
EXCELLENT: add a rule to zlint to auto detect and if possible auto fix the mistake.

Spark specific API

Disable feature

  • dynamic using reconf: preferred
  • env: usually when needed in init, for example port numbers
  • if (0): unittests and non production code
  • TBA: code examples

    etask

    Overview

    etask is Spark's library for writing asynchronous code in a concise synchronous like manner.

    Why etask?

    Promises/async functions don't support structured cancelation, and callbacks are difficult to coordinate/compose.
    etask supports cancelation by default and can manage callback and promise driven subtasks easily.
    For example, if we wanted to find a specific user from the DB, the simplest synchronous code would look like this:

    function user_find_sync(){ let conn = mongodb.connect(); let iter = mongodb.find(conn.users, {}); let u; while ((u = mongodb.get_next(iter))) { if (correct_user(u)) break; } mongodb.close(conn); return u && u.username; }

    But this is synchronous, blocking code. JavaScript is async and single threaded, so blocking calls are a huge performance problem.
    So lets see how to port the sync code to async code. Lets start with the ideal solution, using Spark's etasks and ES6 generators.

    etask ES6 (perfect!):

    let user_find_es6 = ()=>etask(function*(){ let conn = yield mongodb.connect(); let iter = yield mongodb.find(conn.users, {}); let u; while (u = yield mongodb.find_next(iter)) { if (yield correct_user(u)) break; } yield mongodb.close(conn); return u && u.username; }

    Compare this with other possible approaches:

    callbacks (callback-hell...):

    let conn, iter; function user_find_cbs(cb){ mongodb.connect(mongo_connected_cb, cb); } function mongo_connected_cb(cb){ mongodb.find(conn.users, {}, users_find_cb, cb); } function users_find_cb(iter, cb){ mongodb.get_next(iter, filter_users_cb); } function filter_users_cb(u, cb){ if (!u) return mongo_disconnect(cb); correct_user(correct_user_cb, u, cb); } function correct_user_cb(u, is_correct, cb){ if (is_correct) return mongo_disconnect(cb, u.username); mongo_connected_cb(cb); } function mongo_disconnect(cb, username){ mongodb.close(conn, disconnected_cb, cb, username); } function disconnected_cb(cb, username){ cb(username); }

    promise (includes ugly recursion to emulate a loop, nested then, and obscure execution flow):

    function user_find(){ return mongodb.connect() .then(function(conn){ return mongodb.find(conn.users, {}); }) .then(function filter(){ return mongodb.find_next(iter).then(function(u){ if (!u) { return mongodb.close(conn) .then(function(){ return cb(); }); } return correct_user(u).then(function(is_correct){ if (is_correct) { return mongodb.close(conn).then(function(){ user_find_end(user.username); }); } return filter(iter); }); }); }); }

    async function (no support for cancelation if the parent function exits early):

    async function user_find(){ let conn = await mongodb.connect(); let iter = await mongodb.find(conn.users, {}); let u; while (u = await mongodb.find_next(iter)) { if (await correct_user(u)) break; } await mongodb.close(conn); return u && u.username; }

    etask ES5 (when generators are not available):

    function user_find(){ return etask([function(){ return mongodb.connect(); }, function(conn){ return mongodb.find(conn.users, {}); }, function(iter){ return etask.while([function(){ return mongodb.find_next(iter); }, function(u){ if (!u) return this.break(); return correct_user(u); }, function(is_correct){ if (is_correct) this.break(u.username); }]); }, function(u){ username = u; return mongodb.close(conn); }, function(){ return username; }]); }

    Cheat sheet

    synchronous etask ES5 etask ES6
    for this.for() for
    continue this.continue() continue
    return this.return() return

    Usage examples

    Simple calls to etask or promise returning functions:

    let process_items = ()=>etask(function*(){ let items = yield get_items(); for (let item of items) { if (!(yield item_valid(item)) return false; } return true; });

    Call a callback driven function:

    let make_request = url=>etask(function*(){ return yield etask.nfn_apply(request, [url]); });

    Wait on an event emitter:

    let save_request = (req, file)=>etask(function*(){ req.pipe(file) .on('end', ()=>this.continue()) .on('error', e=>this.throw(e)); return yield this.wait(); }); let save_request = (req, file)=>etask(function*(){ req.pipe(file) .on('end', this.continue_fn()) .on('error', this.throw_fn()); return yield this.wait(); });

    Scheduled resource cleanup (like Go's defer statement):

    let do_something = ()=>etask(function*(){ let temp_dir = yield make_temp_dir(); // temp dir will be cleaned up whether the function succeeds or throws this.finally(()=>unlink(temp_dir)); yield do_step1(); yield do_step2(); return yield do_step3(); });

    Coding

    When possible, use ES6 arrow function with no brackets and no return

    let t = function(fn, domain, expected){ let i = 7; return etask(function*(){ ... }; }); let t = (fn, domain, expected)=>etask(function*(){ let i = 7; ... });

    return etask() in the middle of a function should be indented to the function level. Should be used rarely, only when fast path needed

    let get_headers = req=>{ let cache; if (cache = cache_getreq) return cache; return etask(function*get_headers(){ ... }); } let get_headers = req=>{ let cache; if (cache = cache_get(req)) return cache; return etask(function*get_headers(){ ... }); }

    etask class indentation

    class Read_client { search(q){ let _this = this; return etask(function*(){ ... }); } } class Read_client { search(q){ let _this = this; return etask(function*(){ ... }); } } class Read_client { search(q){ let _this = this; return etask(function*(){ ... }); } } class Read_client { search(q){ let _this = this; return etask(function*(){ ... }); } }

    No hidden (automatic) yield in return.

    let insert_cid_to_mongo = cid=>etask(function*(){ let client = yield mongodb.findOne(...cid...); return mongodb.update(...client...); }); let insert_cid_to_mongo = cid=>etask(function*(){ let client = yield mongodb.findOne(...cid...); return yield mongodb.update(...client...); });

    etask name for lib API

    E.find_all = (zmongo, selector, opt)=>etask(function*(){ ... }); E.find_all = (zmongo, selector, opt)=>etask(function*mongo_find_all(){ ... });

    No etask name for internal functions

    let generate_daily = user=>etask(function*generate_daily(){ ... }); let generate_daily = user=>etask(function*(){ ... });

    Avoid enclosing a large portion of a function in a try block.
    Prefer this.on('uncaught', ...) and this.finally when applicable. (why?)

    Code is shorter and the indentation reduce readability.
    let get_zone_bw = config=>(req, res)=>etask(function*(){ let zone = yield check_zone(config, req, res); try { let data = yield get_graphite_bw(req, zone.customer, [zone.name]); res.json(data); } catch(e){ zerr(zerr.e2s(e)); return void res.status(500).send('err'); } }); let get_zone_bw = config=>(req, res)=>etask(function*(){ let zone = yield check_zone(config, req, res); this.on('uncaught', err_handler(res)); let data = yield get_graphite_bw(req, zone.customer, [zone.name]); res.json(data); });

    React code

    Read React docs and follow guidlines and recommendations unless they conflict with Spark React conventions.

    Use Components to stay DRY

    Use the same guidlines for code repetition as regular functions: Try not repeat yourself.

    Move shared JSX to a util component to repeat markup

    <div className="feature"> <h3>title1</h3> <p>text</p> </div> <div className="feature"> <h3>title2</h3> <p>text2</p> </div> let Feature = props=> <div className="feature"> <h3>{props.title}</h3> <p>{props.children}</p> </div>; <Feature title="title1">text1</Feature> <Feature title="title2">text2</Feature>

    JSX

    JSX coding convention follows HTML coding. When switching from JS code to JSX code use 4 chars indentation on the first. Rest follow HTML convention of 2 chars indentation.

    return <View> </View>; return <View></View>; return <View> </View>; return <View> <Button/> </View>; return ( <View> <Button/> </View>); return ( <View> <Button/> </View> ); return ( <View> {show_panel && <Panel/>} </View>) ); return ( <View> {show_panel && <Panel/>} </View> );

    CSS

    Use CSS conventions for react

    Unittest

    TBD