it-is-war

TL;DR: Shocking code and why else-if is evil. The worst most abusive use of else-if I’ve ever seen

This is really bad

In part-one I mentioned that once you start with else-if it can be hard to stop. Below is an example of just that.

Good Code first

Just an array of regular expressions, and then a string to match with. The choosen regex depends on the length of the string.

  • Refactor if-else to table-driven approach
   validate(nino: string) {
        const partialNinoRegEx = [
            /^[A-Z]{1}/i,
            /^[A-Z]{2}/i,
            /^[A-Z]{2}\d{1}/i,
            /^[A-Z]{2}\d{2}/i,
            /^[A-Z]{2}\d{3}/i,
            /^[A-Z]{2}\d{4}/i,
            /^[A-Z]{2}\d{5}/i,
            /^[A-Z]{2}\d{6}/i,
            /^[A-Z]{2}\d{6}[A-Z]{1}/i];

     return partialNinoRegEx[nino.length + 1].test(nino);

Bad Code - Warning Will Robbinson

Yeah I don’t know what to say about this one

The part I like the most is the “return error”, it’s funny because, I don’t see a return statement!!! :-). I think the engineer just got into the else-if flow and that was it. Obviously this was written by a very junior developer, so they can be forgiven.

However, it was not written by a junior developer….. think a little higher…. senior…. higher… lead software engineer. It just shows you don’t have to know how to code to become a lead software engineer.

   scope.ninoValidation = function (nino) {
          if (nino === undefined || nino === null) {
            scope.ninoValid = false;
          } else {
            if (nino.length === 1) {
              scope.ninoValidationRule = /^[A-Z]{1}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 2) {
              scope.ninoValidationRule = /^[A-Z]{2}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 3) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{1}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 4) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{2}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 5) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{3}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 6) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{4}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 7) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{5}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 8) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{6}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else if (nino.length === 9) {
              scope.ninoValidationRule = /^[A-Z]{2}\d{6}[A-Z]{1}/i;
              scope.ninoValid = scope.ninoValidationRule.test(nino);
            } else {
              //Return error
              scope.ninoValid = false;
            }
          }
​