High Quality Code Standard and Pull Request guidelines

February 16th, 2021

High quality code standard

The purpose of creating high quality code is to best ulitilize computer resources (CPU, RAM, bandwidth), to minimize development cost (shorter pr, time to review, maintain), to create a consistent high quality code base and maintain positive team member's development experience:

Pull request review guidelines

Pull request creation guidelines

Correct, short and simple

Correct: your code must correctly satisfy ticket requirements.

Short: with the same correctness, code must be as short as possible.

Simple: if there are 2 solutions which are both correct & short, always choose the simpler one. Never make things complicated. The software engineering itself is already complicated.

❝The code that's the fastest to write, the code that's the easiest to maintain and the code that never breaks, is the code you don't write. So that's our strategy, write a lot less code.❞ - Steve Jobs

// Example: write a function to return a sum of two numbers

// bad: 4 lines of code
function sumOfTwoNumber(a, b) {
  const sum = a + b
  return sum
}

// good: 1 line of code, same correctness
const sum = (a, b) => a + b

Secured

Secured code: never run code on our server at the will of caller.

// Example: populate database

// bad: query and populate are client code, executed on our server at user's will
async getUser(payload, done) {
  const { query, populate } = payload
  const user = User.find(query).populate(populate)
  done({ success : true, user})
}

// good: db query execute on server logic with only caller's information
async getUser(payload, done) {
  const { query : { email }, populate: { populateProperty } } = payload
  const populate = populateProperty ? { path: 'propterty' } : {}
  const user = User.find({ email }).populate(populate)
  done({ success : true, user})
}

Optimized time, space and bandwidth

Always calculate the complexity and bandwidth of your code with Big-O notation. Try optimize your code within good and excellent area.

Optimized CPU, RAM: always question if the current complexity is optimized.

Optimized bandwidth: always question if you are sending any unnecessary data.

Testable

Your code must be testable with unit test. And the test setup should be as simple as possible.

Always test with multiple corner cases on developement and staging environment to minimize production bugs.

Easy to review and maintain

Reviewers can quickly understand what your implementation is. If someone (or you) needs to come back and maintain the code, he/she can quickly understand and modify it at will.

Sematic variable and function naming

Good variable naming: short, reflects the data it holds

// bad
const validationData = true

// good
const isValidated = true

Good fuction/method naming: short, reflects what it does. Remember to use active verb, NOT passive form to start the name of a function/method.

// bad
const logNotificationError = (err) => {
  log.error(err)
}

// good
const logFailedPushError = (err) => {
  log.error(err)
}

Well IPO documented and step by step commented

Always document code per IPO guidelines. IPO helps to understand what a function does without reading every line of the function. If you don't IPO your code, reader may have to read every line of related functions and the functions within those functions, just to know what they do. That action wastes a lot of time.

// bad
const sum = ({a , b}) => a + b

// good
// input: obj.a: base trading rate
//        obj.b: realtime volativity adjustment
// output: real time trading rate
// process: sum function add up base and adjustment to return realtime
//          stock trading rate
const sum = ({a , b}) => a + b

Always explain the process with comment along your code so reviewer can quickly understand your intention, and maintainer can quickly find the logic he/she needs to update. Remember, code is implementation of a solution, code itself is NOT a solution. That's why you need to comment to capture your solution.

// bad
const sum = ({a , b}) => {
  const c = getElixir(a)
  const d = oxi(b)
  return c + d
}

// good
// input: obj.a: base trading rate
//        obj.b: realtime volativity adjustment
// output: real time trading rate
// process: sum function add up base and adjustment to return realtime
//          stock trading rate
const sum = ({a , b}) => {
  // check historical data to get average base
  const c = getElixir(a)

  // apply averaging algorithm to get real time adjustment
  const d = oxi(b)

  return c + d
}

Comments and IPO documentation does not count as code execution lines.

Elegant

Are you proud of your code? Do you think 1 months from now, when you look back at it, you'll say: "what a beautiful piece of work". Do your peers tell you:

Your code is easy, pleasant to read and review.
Your solution is short, precise and secured.
I just want to approve your code right at the first time I review it.

Those are some indicators that say your code is elegant, high in quality.

Javascript styleguide, use AirBnB's

Follow AirBnB Javascript style guide when creating and reviewing PR: https://github.com/airbnb/javascript

Pull request review guidelines

The purpose of PR review is to strengthen the high quality code standard by running it again with an outsider point of view. In order to contribute meaningful comments, the reviewer must:

Strictly follow high code quality standard when making reviews.
Always start a comment with a code standard. Example: CS2: this structure will create a vulnerbility of database injection. I suggest we should ...
To save time, reviews must come in rounds, not in separate comments.
The goal for discussions or debates must be high code quality standard.
Keep an open mind, nothing is personal, every team member goal is to have a good product.
Be humble when presenting your opinions.

Pull request creation guidelines

Discuss and agree on solution with reviewer(s) before implementation.
Check off all PR checklist ✅ before request for review.
Visually present your work (especially FE + Mobile PRs) with photos/gif/vid so reviewers have a better sense of your work.
Review your PR before requesting reviewers.
Resolve all reviewers' comments before requesting next round.
Prepare to have your work criticized (take it in a constructive way). It's the nature of our industry. Any software product, any PR will have feedbacks and comments. If your PR has no comments and straight approvals, you know what you have written a good PR.
Keep an open mind, nothing is personal, every team member goal is to have a good product.
Be humble when presenting your opinions.