2019年8月13日 星期二

Take advantage of variables

I think those two concepts (maybe call them philosophy) are the most useful techniques in Object-Oriented Design.

  • DRY: Extract repeated or similar functions and reuse them.
  • Single Responsibility: Keep your small chunk of code simple.

I would like to add one more thing that I usually call three of them "Good Code Smell"

  • Take advantage of variables.

We normally write code directly like this:

function checkScore {
  if (score > 50) {
    // do something
  }
}

When the condition becomes complicated:

function checkScore {
  if (score > 50 && score <= 100) {
    // do something
  }
}

More complicated:

function checkScore {
  if (score > 50 && score <= 100 && seconds > 60 && seconds <= 120) {
    // do something
  }
}

Well, all of them look ok, but how about this one:

function checkTenPercentOff() {
  if (
    // check qualified user
    user.title == 'Gold Member' && user.points > 1000 &&
    // check cart is not empty
    cartItems != null && cartItems != undefined && cartItems.length > 0 &&
    // check each item's quantity
    checkQuantity(cartItems) &&
    // check special sales period
    now > new Date('2019-09-01T00:00:00') && now <= new Date('2019-12-31T00:00:00')
  ) {
    // do 10% off for the user
  }
}

This is a monster if statement although it may still simple compared with real requirements from an SRS.

Now, we can take advantage of variable names.

function checkTenPercentOff() {
  // Look, we even don't need comments if we have good variable names
  var isQualifiedUser = user.title == 'Gold Member' && user.points > 1000;
  var isNotEmptyCart = cartItems != null && cartItems != undefined && cartItems.length > 0;
  var allItemsHaveQuantity = checkQuantity(cartItems);
  var isSpecialSalesPeriod = now > Date('2019-09-01T00:00:00') && now <= new Date('2019-12-31T00:00:00');

  var isQualifiedForTenPercentOff =
    isQualifiedUser &&
    isNotEmptyCart &&
    allItemsHaveQuantity &&
    isSpecialSalesPeriod;

  if (isQualifiedForTenPercentOff) {
    // do 10% off for the user
  }
}

It looks much clear and easy to read now.

Remember the DRY and Single Responsibility? How about this:

// extract variables as parameters, then you don't hard-code a function.

function checkQualifiedUser(title, points) {
  var isQualifiedUser = user.title == title && user.points > points;
  return isQualifiedUser;
}

function checkNotEmptyCart(cartItems) {
  var isNotEmptyCart = cartItems != null && cartItems != undefined && cartItems.length > 0;
  return isNotEmptyCart;
}

function checkAllItemsHaveQuantity(cartItems) {
  var allItemsHaveQuantity = checkQuantity(cartItems);
  return allItemsHaveQuantity;
}

function checkSpecialSalesPeriod(startDate, endDate) {
  var isSpecialSalesPeriod = now > startDate && now <= endDate;
  return isSpecialSalesPeriod;
}

function checkTenPercentOff() {
  var isQualifiedForTenPercentOff =
    checkQualifiedUser('Gold Member', 1000) &&
    checkNotEmptyCart(cartItems) &&
    checkAllItemsHaveQuantity(cartItems) &&
    checkSpecialSalesPeriod(new Date('2019-09-01T00:00:00'), new Date('2019-12-31T00:00:00'));

  if (isQualifiedForTenPercentOff) {
    // do 10% off for the user
  }
}

function checkFifteenPercentOff() {
  var isQualifiedForFifteenPercentOff =
    checkQualifiedUser('Diamond Member', 2000) &&
    checkNotEmptyCart(cartItems) &&
    checkAllItemsHaveQuantity(cartItems) &&
    checkSpecialSalesPeriod(new Date('2020-01-01T00:00:00'), new Date('2020-06-31T00:00:00'));

  if (isQualifiedForFifteenPercentOff) {
    // do 15% off for the user
  }
}

Now, we have reusable(DRY) and short(Single Responsibility) functions and clear variable names(Take advantage of variables)