CODE QUALITY

What do we mean by "quality?"

Decisions that affect human readability

 

  • Includes layout choices, such as spacing and punctuation
  • Includes lexical choices, such as use of optional keywords and shorthand syntax

Decisions that impact reasonability, maintainability and performance

 

  • Separation of concerns and modularity
  • DRY
  • Test coverage
  • Comments / inline documentation
  • Avoid global variables 
  • Error handling

Why does quality matter?

  • Durability
    • As you update your codebase, unintended consequences (bugs!) are kept to a minimum
  • Collaboration
    • Make your code easy to read and reason about
    • New team members can contribute more effectively

Use a Style Guide

What matters more?

Style choices you make

Being consistent

if (!item) return { error: true, message: "No record found" };
if (!item) 
  return { error: true, message: "No record found" };
if (!item) {
  return { error: true, message: "No record found" };
}

OR

Subtle differences matter - can you spot them?

Clean Code Javascript

Linters are Your Best Friend

Below examples are valid JavaScript, but disobey predetermined style rules 

ESlint - Airbnb Style Guide

$ npm i eslint -g

$ eslint --init
module.exports = {
    "extends": "airbnb",
    "parser": "babel-eslint",
    "plugins": [
        "react",
        "jsx-a11y",
        "import",
        "flowtype"
    ],
    "rules": {
        "react/jsx-filename-extension": 0,
        "react/prefer-stateless-function": 1,
        "arrow-parens": 0
    }
};

Make It Work... then Refactor

Starting with an inline anonymous function

Objective: Use a custom Mongoose model validator

to verify model "Book" has a unique "title" field value

const BookSchema = new Schema({
    title: {
        type: String,
        validate: function(val, callback){
            mongoose
                .model('Book')
                .findOne({ title: val })
                .then( book => callback(book === null) );
        }
    }
});

Make It Work... then Refactor (2)

Extract to a function inside a utility module

const { bookUniqueTitleValidator } = require('./utils/validators');

const BookSchema = new Schema({
    title: {
        type: String,
        validate: bookUniqueTitleValidator
    }
});

// utils/validators.js

exports.bookUniqueTitleValidator = function(val, callback){
    mongoose
        .model('Book')
        .findOne({ title: val })
        .then( book => callback(book === null));
};

Make It Work... then Refactor (3)

Improve String validator to work with ANY model/field

const { uniqueStrValidator } = require('./utils/validators');

const BookSchema = new Schema({
    title: {
        type: String,
        validate: uniqueStrValidator('Book', 'title')
    }
});

How could this be improved further?

// utils/validators.js

exports.uniqueStrValidator = function(model, fieldName){
    return function(val, callback){
        mongoose
            .model(model)
            .findOne({ [fieldName]: val })
            .then( item => callback(item === null) );
    };
};

Helping Hand with Code Metrics extension (VSC)

This function has more than a single responsibility. It is:

1) inserting a database record

2) mapping an additional property onto every object in the response

Let's extract the mapping process to its own function

File organization / folder hierarchy

  • Separate client and server (if fullstack)
  • One "class" (or function constructor) per file
  • Similar purpose functions should be grouped into modules
  • (REACT) Break out different categories of actions and reducers into separate files
  • Always retrieve "secrets" from server-side
    • Remember all client code is visible to anyone who visits your web app
    • On server, store secrets in environment variables
      • ​handy npm package: dot-env
      • secrets should not be stored in your git repo
      • secrets should be separated for environment (dev, staging, prod...)

Interfaces

In the same way you expect a public API to be stable, remember every new function or method you write is an interface for other places in your codebase.

 

You or a collaborator have expectations that an interface will change infrequently

 

A function that handles implementation details (aka. private interface) is usually called from less sources and could change more often

// `createClass` will always be available even 
// if behind-the-scenes implementation changes

React.createClass({
    render(){
    }
});

// `findAllOnSale` will always be available even if 
// behind-the-scenes implementation changes

Book.findAllOnSale(date);

...prototype.findAllOnSale = function(date){
  // calls internal function(s) that handle
  // implementation -- input / output doesn't change
};

React API

Custom Public Interface

Github Readme

  • Screenshot
  • Short overview
    • ​What does the app do?
    • Who is it for?
  • ​Requirements
    • ​Specs tested on
    • Prerequisite software
  • ​Installation
    • ​Step by step to install to local dev environment
  • ​Usage
    • ​Command(s) to run in dev mode
    • Options
  • License
  • How to Contribute
  • Acknowledgements

Reminders

  • Follow ES6 standards
  • Don't over-rely on libraries/APIs
  • 100% test coverage
  • Good variable names
  • Avoid global variables
  • Is it DRY?
  • Can any logging or debugging code be removed?
  • Are all functions commented?
  • If there is incomplete code, is it marked TODO?
  • Are you handling unexpected user behavior?
  • Avoid nested `if`s
  • React: Default to functional components
  • Semantic HTML / forms?
  • All images have alt tags?
  • Clear element hierarchy
  • Avoid 'div soup'

JavaScript

HTML

  • Class names should be descriptive
  • Fallbacks for font-family
  • Appropriate prefix use

CSS

Code Quality

By Thinkful

Code Quality

Deep dive into good code quality principles

  • 1,090

More from Thinkful