Eliminate JavaScript Code Smells

Have you ever written JavaScript that reeked, but you couldn’t quite figure out why? Or have you written code and immediately knew that it wasn’t good, but didn’t know a better way?

Your JavaScript may pass stardard JSHint rules, but that doesn’t mean it’s legit.

“A code smell is a surface indication that usually corresponds to a deeper problem in the system” –Martin Fowler

Most developers can smell brittle and fragile code a mile away, but it takes time and training to combat against these smells. Code smells scream to be refactored.

In this session we will discuss various common smelly code snippets and discuss techniques on how we can eliminate and protect against their pungent odors creeping into your codebase.

Convoluted Code Smell – too many statements, too much depth, too complex. But what does “complex” mean? JSHint can tune for these factors and give a complexity score. It can throw errors when there are too many arguments or callbacks, etc… Then when you write tests and refactor, you can put numbers on the reduction in complexity.

Copy Paste Code Smell – duplication in code. There are tools like Jsinspect, JSCPD, CodePen which can detect copy+paste code. You can adjust how exactly it needs to match.

Switch Statement Smell – when code violates the Open/Closed Principle (OCP), one of Uncle Bob’s SOLID Principles. Instead you can use the Strategy Design Pattern to add new code without touching existing code. Sadly… no tooling for this.

Edge cases? ESLint and JSHint can be disabled for a few lines if you need to.

The This Abyss – this=that, etc. Alternatives… bind, second param on forEach, ES6 fat arrow =>…

Long string concatenation smell – ugly string concatenation with lots of quoting. Alternatives? Tomas Fuchs tweet sized js templating engine. ES6 template strings, including multiline! Or use a full templating or mvc option.

Jquery Inquiry – excessive use of fluent code, where everything is chained. Problem is silent error handling vs long chains of code. Instead refactor out to use delegated events. Put less in document ready.

Temperamental Timer Smell – use of setInterval; which then gets out of sync. Use settimeout with a callback instead, so you can’t get out of sync.

Repeat Reassign Smell – repetition. Lots of options, although many feel just as repetitive… Using reduce works… but ended up using lodash flow.

Incessant Interaction Smell – code that needs to be debounced (autocompletes, listening to scroll events, etc). This is better than throttling as it waits for a pause from the user before making calls.

Anonymous Algorithm Smell – when there are anonymous functions used in ways that mess up stack traces, code reuse, etc. Wherever you currently use an anon function, you can name it.

Last thoughts… Eslint is pluggable, so if you want to test something in particular you can write your own (or just as likely someone else has already done that). There are plugins for angular, react, backbone… npm search for eslint plugins!

(lots of code in this one, if you are interested look up the slides)

elijahmanor.com/talks/js-smells