Guidelines and best practices used by Eventbrite to provide consistency and prevent errors in any environment where JavaScript code is written.
Complex conditional expressions within conditional statements can make understanding code challenging because you need to fully understand the expression in order to be able to process the overall code flow. In addition, in code reviews (or revisiting old code), it's difficult to know if the logic is correct without knowing what state the conditional expression is trying to represent.
To make things easier, store complex conditional expressions in state variables:
// good
var shouldContinue = !options || (options.continue && options.hasWon);
if (shouldContinue) {
}
// bad
if (!options || (options.continue && options.hasWon)) {
}Positive conditional expressions are generally easier to follow than negative ones. Try to flip the logic so that the positive case is considered first:
// good
if (something && somethingElse) {
/* do stuff*/
} else {
/* do other stuff */
}
// bad (negative case comes first)
if (!something || !somethingElse ) {
/* do stuff */
} else {
/* do other stuff */
}Creating a chain of conditional statements can make refactoring hard because the decisions are mixed with the actions. Instead, separate out the decisions into functions that can return simple actions:
// good
// decision
var whatToDoWithTheThing = this.getSomethingAction();
// execution
this[whatToDoWithTheThing]();
// bad (mixed decisions and actions)
if (this.something()) {
/* do stuff */
} else if (this.otherSomething()) {
/* do other stuff */
} else if (this.somethingEvenMoreCurious()) {
/* yet another thing */
} else {
/* default something */
}For more details, check out Replacing the switch statement for object literals.
First iteration: Multiple return statements and if-else statements that mix decision with execution.
var getType = function(model) {
if (model % 2 !== 0) {
return 'odd';
} else if (model % 2 !== 0) {
return 'even';
} else if (model === 'Eventbrite') {
return 'Eventbrite';
}
return 'default';
};Second iteration: A lookup map helps avoid using more than one return statement inside a given function and abstracts away the conditional logic. This makes it easier for maintainability and readability, as well as helping with performance by using .find.
// good (use a lookup map)
var helpers = {
isOdd: function(value) {
return value % 2 !== 0;
},
isEven: function(value) {
return value % 2 === 0;
},
isEventbrite: function(value) {
return value === 'Eventbrite';
}
}
var types = {
isOdd: 'odd',
isEven: 'even',
isEventbrite: 'eventbrite'
};
_getType = function(types, model) {
var type = _.find(types, function(value, key) {
// will return the first value of the key that evaluates to true;
return helpers[key](model);
}
return type || 'default';
}NOTE: We are using Underscore's .find method here, but with ES6, we can use find natively. For more info, visit https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/find.
Simple ternary expressions can be handy when we need to conditionally assign a value to a variable when the condition is both true and false. However, when we only want to assign the variable when the condition is true, we may be tempted to still use a ternary and return undefined, null, '' for the false case.
In this case, the preferred approach is to declare the value without assigning it (the default value is undefined) and then using an if statement to assign to it when the condition is met:
// good
var value;
if (options.isSomethingTrue) {
value = 'hey there';
}
return value;
// bad (uses a ternary that returns undefined or null)
var options.isSomethingTrue ? 'hey there' : undefined;Applying this pattern provides more robust maintenance and traceability (in debugging) over time and uses the basic structures of the language in a more formal way (ternaries are used under the assumption that we need a value).
This correlates as well with a defined state and later on if needed altering such state instead of having an default state that is dynamic.
Let's look at a case study of how code can evolve over time. For this example we have created a fictional piece of code.
First iteration: nothing fancy, just a method that needs to gather some information:
var isAllowed = hasParent && isOwner,
parentEvent = isAllowed ? getParentEvent() : undefined;
/* some use of these 2 variables later on... */Second iteration: another developer comes around and adds yet another condition following the current style given that a refactor is not really needed, and probably out of scope:
var isAllowed = hasParent && isOwner,
parentEvent = isAllowed ? getParentEvent() : undefined,
options = isAllowed ? undefined : getOptions();
/* some use of these 3 variables later on... */Third iteration: another team comes in needing more functionality and adds even more variables:
var isAllowed = hasParent && isOwner,
parentEvent = isAllowed ? getParentEvent() : undefined,
options = isAllowed ? undefined : getOptions(),
childEventsTitles = isAllowed ? getEventChildTitles() : undefined,
ownerAccount = isAllowed ? undefined : getOwnerAccount();
/* some use of these 5 variables later on... */At this point, telling what is the base state for this method is quite hard, given that all the possible states are based on ternaries. Furthermore, because we use the same isAllowed four times, we have to know how all the state variables work in order to try optimize the code. The code is too fragile.
However, if this code would have followed the initial recommendation, the code wouldn't degrade as more functionality is added.
First iteration (revisited):
var isAllowed = hasParent && isOwner,
parentEvent;
if (isAllowed) {
parentEvent = getParentEvent();
}
/* some use of these 2 variables later on... */Second iteration (revisited):
var isAllowed = hasParent && isOwner,
parentEvent,
options;
if (isAllowed) {
parentEvent = getParentEvent();
} else {
options = getOptions();
}
/* some use of these 3 variables later on... */Third iteration (revisited):
var isAllowed = hasParent && isOwner,
parentEvent,
options,
childEventsTitles,
ownerAccount;
if (isAllowed) {
parentEvent = getParentEvent();
childEventsTitles = getEventChildTitles();
} else {
options = getOptions();
ownerAccount = getOwnerAccount();
}
/* some use of these 5 variables later on... */Avoid using IFFEs (immediately-invoked function expressions) if possible. The bind function is preferred when dealing with variable scope in closures (note that bind is only available in ES5+,
though we use es5-shim so bind should always be in our codebase by default).
var purchaseTicketButtonSelector = '.goog-te-combo';
// good (using .bind)
buyTicketFunction = function(options) {
buyTickets(_.pick(options, 'eventId', 'ticketId'));
};
$(purchaseTicketButtonSelector).click(buyTicketFunction.bind(null, options));
// bad (using IFFEs)
buyTicketFunction = (function(options) {
return function() {
buyTickets(_.pick(options, 'eventId', 'ticketId'));
};
})(options);
$(purchaseTicketButtonSelector).click(function() {
buyTicketFunction();
});IFFEs tend to add unnecessary complexity (note that the "bad" IFFE example has three nested functions, while the "good" example has one) and is harder to change or extend.
Avoid variable indirection when possible. Reassigning variables when no transformation is needed creates unnecessary indirection, which is prone to introduce future errors.
// good
_handleEvent = function(e) {
_doSomethingWithEvent(e.target.checked);
}
// bad
_handleEvent = function(e) {
var checked = e.target.checked;
_doSomethingWithEvent(checked);
}There are only two hard things in Computer Science: cache invalidation and naming things. Phil Karlton
Classes or newable functions (meant to be factories) should be singular, camelCase, and begin with a capital letter:
var groupTicket = new Ticket(id: 555);Any variable that's not a class or newable function, should be camelCase. This includes local variables, helper functions, modules, etc.
//good
var pagination = require('pagination'),
pages = pagination.getPages();
//bad
var Pagination = require('pagination'),
pages = Pagination.getPages();Variables that will contain a boolean value should start with either is or has:
//good
var isAvailable = true,
hasTickets = false;
//bad
var available = true,
IHaveTickets = false;Variables that contain jQuery elements should start with a dollar sign ($) so that we can quickly identify these types of variables by scanning the code (i.e. static analysis). This makes code review easier.
var $elements = $('someSelector'), // in case a global search
$element = this.$('some selector'); // in case of backboneIf a variable contains a jQuery.Deferred() or Promise, the variable should end in Dfd or Promise, respectively:
//good
var fetchPromise = fetch('http://some/url'),
requestDfd = new $.Deferred() // in case of jquery promises.
//bad
var request = fetch('http://some/url'),
onResolve = new $.Deferred();Constats should be all written in capital letters and in snake_case. (except config object.)
//good
const DEFAULT_VALUE = 'none';
var TIMEOUT = 3;
var config = {};
//bad
const defaultValue = 'none';
var timeout = 3;
var someOtherStaticVariable = 'blah';Avoid names that are generic and instead, try to make your best effort to find a proper name in order to explain the content of it. If data needs to be changed, as part of a process, try to create methods to produce this changes on demand.
//good
var attendeeNames = {},
translatedGreeting 'hola!',
getDefaultPrice = parseInt(5, 10);
//bad
var propsObject = {},
trimmedString = 'hello '.trim(),
intPrice = parseInt(5, 10);Prefix private method names with an underscore (_):
// good
var _getInternalValue = function() {
// code here.
};
// bad
var getInternalValuePleaseDonot = function() {
// code here.
};Using the underscore prefix naming convention for private methods allows us to easily identify which methods are not part of the component's public API. As a result, future developers can easily know which methods can be refactored or even removed without affecting any users of the component. This makes upgrading to new technologies/tools/idioms simpler.
The naming convention also helps code reviewers to quickly understand which methods have been added by us versus which are a part of the framework's lifecycle methods (see: react or Marionette).
should always begin with a lowerCase
//good
getNode: function() {
//code here.
},
setAttr: function() {
// code here.
};
//bad
_superComplextName: function() {
// code here.
}
UpdateAll: function() {
/ /code here
}