Introduction

Throughout this series of articles, we have presented programming techniques that allow us to generate more maintainable code. Most programming techniques and recommendations come from the "Clean Code" book and from the application of these techniques over years of experience.

In this article, I am going to describe, step by step, the application of refactoring techniques that I have applied to a code written for one of my programming fundamentals classes. If you are starting to develop software, my recommendation is that you try to solve the problem with the techniques and tools you know first (we will use JavaScript as programming language). In case you already have programming knowledge and solving the problem is not a great effort, the exercise is different. In this case, a solution is provided, the starting code, and the challenge is to apply different refactoring techniques to understand the code in depth and make this code more maintainable.

For this challenge, I have prepared a GIT repository in which you find all the versions of the algorithm that we are going to solve throughout the post step by step, using JavaScript, and a series of npm-scripts that allow you to execute the code in each one of these steps using the following nomenclature:

npm run stepX # Where X is the step

The GIT repository where you can find the code is as follows: REPOSITORY.

The Problem: Ceaser Cipher

The description of the problem is extracted from Wikipedia. So, you can read more from the original source.

Caesar Cipher, is one of the simplest and most widely known encryption techniques. It is a type of substitution cipher in which each letter in the plaintext is replaced by a letter some fixed number of positions down the alphabet. For example, with right shift of 3, E would be replaced by H, F would become I, and so on.

The transformation can be represented by aligning two alphabets; the cipher alphabet is the plain alphabet rotated right by some number of positions. For instance, here is a Caesar cipher using a right rotation of six places, equivalent to a right shift of 6:

Plain:    ABCDEFGHIJKLMNOPQRSTUVWXYZ
Cipher:   GHIJKLMNOPQRSTUVWXYZABCDEF

When encrypting, a person looks up each letter of the message in the "plain" line and writes down the corresponding letter in the "cipher" line.

Plaintext: THE QUICK BROWN FOX JUMPS OVER THE LAZY DOG Ciphertext: QEB NRFZH YOLTK CLU GRJMP LSBO QEB IXWV ALD

Deciphering is done in reverse, with a left shift of 6.

What's and Why Refactoring?

Refactoring is a well-known topic in the software development industry. At this point, we make an introduction to the topic, but I recommend that you read the following article: https://www.cuelogic.com/blog/what-is-refactoring-and-why-is-it-important. From this article, we extract the main ideas which we are going to share here.

Refactoring is no Silver Bullet but it is a valuable weapon which benefit you to keep excellent hold on your code & so the project (software/application).

It is a scientific process of taking existing code and improves it while it makes code more readable, understandable, and clean. Also, it becomes very handy to add new features, build large applications and spot & fix bugs.

Reasons why Refactoring is Important:

To improve the design of software/application.
To make software easier to understand.
To find bugs.
To fix existing legacy database.
To provide greater consistency for user.

Original code

Once we know the problem we want to solve, we carry out an implementation that anyone who is starting out in development can reach with little time.

function cipher(text, shift) {
  var cipher = '';
  shift = shift % 26;
  for (var i = 0; i < text.length; i++) {
    if (text.charCodeAt(i) >= 65 && text.charCodeAt(i) <= 90) {
      if (text.charCodeAt(i) + shift > 90) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) + shift - 26),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
      }
    } else if (text.charCodeAt(i) >= 97 && text.charCodeAt(i) <= 122) {
      if (text.charCodeAt(i) + shift > 122) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) + shift - 26),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
      }
    } else {
      // blank space
      cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
    }
  }
  return cipher.toString();
}

function decipher(text, shift) {
  var decipher = '';
  shift = shift % 26;
  for (var i = 0; i < text.length; i++) {
    if (text.charCodeAt(i) >= 65 && text.charCodeAt(i) <= 90) {
      if (text.charCodeAt(i) - shift < 65) {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift + 26),
        );
      } else {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift),
        );
      }
    } else if (text.charCodeAt(i) >= 97 && text.charCodeAt(i) <= 122) {
      if (text.charCodeAt(i) - shift < 97) {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift + 26),
        );
      } else {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift),
        );
      }
    } else {
      // blank space
      decipher = decipher.concat(
        String.fromCharCode(text.charCodeAt(i) - shift),
      );
    }
  }
  return decipher.toString();
}

The code we want to develop has two methods:

cipher - Which will take the text and the shift to apply in one direction.
decipher - Performs the opposite operation of cipher. That is, decipher the text.

I recommend that, whenever you go to perform refactoring on code, you have a series of automated tests to help you verify that you have not "broken" the code. In this specific case, instead of creating a test suite, I have created two checks using the standard console.assert.

Therefore, the checks to know whether the algorithms are stable will be done through the following asserts.

console.assert(
  cipher('Hello World', 1) === 'Ifmmp!Xpsme',
  `${cipher('Hello World', 1)} === 'Ifmmp!Xpsme'`,
);
console.assert(
  decipher(cipher('Hello World', 3), 3) === 'Hello World',
  `${decipher(cipher('Hello World', 3), 3)} === 'Hello World'`,
);

Well, we already have the challenge that we are going to carry out, let's start playing!

Step 1. Magic numbers

The first step is to remove the magic numbers that appear in the code by a variable name that gives semantic value to the code. In this way, the following numbers would be modified:

The number of letters in our alphabet (26).
Each letter that belongs to the limits where the algorithm should be circular, that is:
- a: 65.
- z: 90.
- A: 97.
- Z: 122.

Therefore, we define the following constants that will allow us to have a semantic context of what each of these numbers represents.

const NUMBER_LETTERS = 26;
const LETTER = {
  a: 65,
  z: 90,
  A: 97,
  Z: 122,
};

In this way, the code will be as follows after this change.

const NUMBER_LETTERS = 26;
const LETTER = {
  a: 65,
  z: 90,
  A: 97,
  Z: 122,
};

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    if (text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z) {
      if (text.charCodeAt(i) + shift > LETTER.z) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) + shift - NUMBER_LETTERS),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
      }
    } else if (
      text.charCodeAt(i) >= LETTER.A &&
      text.charCodeAt(i) <= LETTER.Z
    ) {
      if (text.charCodeAt(i) + shift > LETTER.Z) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) + shift - NUMBER_LETTERS),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
      }
    } else {
      // blank space
      cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
    }
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    if (text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z) {
      if (text.charCodeAt(i) - shift < LETTER.a) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift + NUMBER_LETTERS),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) - shift));
      }
    } else if (
      text.charCodeAt(i) >= LETTER.A &&
      text.charCodeAt(i) <= LETTER.Z
    ) {
      if (text.charCodeAt(i) - shift < LETTER.A) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift + NUMBER_LETTERS),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) - shift));
      }
    } else {
      cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) - shift));
    }
  }
  return cipher.toString();
}

console.assert(
  cipher('Hello World', 1) === 'Ifmmp!Xpsme',
  `${cipher('Hello World', 1)} === 'Ifmmp!Xpsme'`,
);
console.assert(
  decipher(cipher('Hello World', 3), 3) === 'Hello World',
  `${decipher(cipher('Hello World', 3), 3)} === 'Hello World'`,
);

Step 2. Extract similar code from if-else

The next step is to identify those lines of code that are repeated in the code, so that these lines can be extracted into functions. Specifically, the assignments that exist in the bodies of the if control structures are repeated throughout the code, and these can be extracted.

That is, the following code snippet cipher = cipher.concat (String.fromCharCode ( can be extracted from the different if's that exist in the code. This line is executed after the if structure while the if that only contain the different logic according to each one of the cases.

Of course, the same operations that we perform for the cipher function are performed for the decipher function.

The code after of apply this refactoring is the following:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;

  for (let i = 0; i < text.length; i++) {
    let character = '';
    if (text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z) {
      if (text.charCodeAt(i) + shift > LETTER.z) {
        character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) + shift;
      }
    } else if (
      text.charCodeAt(i) >= LETTER.A &&
      text.charCodeAt(i) <= LETTER.Z
    ) {
      if (text.charCodeAt(i) + shift > LETTER.Z) {
        character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) + shift;
      }
    } else {
      // blank space
      character = text.charCodeAt(i) + shift;
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character = '';
    if (text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z) {
      if (text.charCodeAt(i) - shift < LETTER.a) {
        character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) - shift;
      }
    } else if (
      text.charCodeAt(i) >= LETTER.A &&
      text.charCodeAt(i) <= LETTER.Z
    ) {
      if (text.charCodeAt(i) - shift < LETTER.A) {
        character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) - shift;
      }
    } else {
      character = text.charCodeAt(i) + shift;
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Step 3. Avoid else

The next step is to avoid the code related to the else control structure block. Avoid it is quite easy since we simply have to move the code from the else to the variable character before the beginning of the loop so that this value is assigned as the default value.

Therefore, the code after this refactoring is as follows:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character = text.charCodeAt(i) + shift;
    if (text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z) {
      if (text.charCodeAt(i) + shift > LETTER.z) {
        character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) + shift;
      }
    } else if (
      text.charCodeAt(i) >= LETTER.A &&
      text.charCodeAt(i) <= LETTER.Z
    ) {
      if (text.charCodeAt(i) + shift > LETTER.Z) {
        character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) + shift;
      }
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character = text.charCodeAt(i) + shift;
    if (text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z) {
      if (text.charCodeAt(i) - shift < LETTER.a) {
        character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) - shift;
      }
    } else if (
      text.charCodeAt(i) >= LETTER.A &&
      text.charCodeAt(i) <= LETTER.Z
    ) {
      if (text.charCodeAt(i) - shift < LETTER.A) {
        character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
      } else {
        character = text.charCodeAt(i) - shift;
      }
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Step 4. Merge the IF logic

The next step is tortuous for us but we have to merge the logic corresponding to the if-elseif. So that, we only have two control structures if. This action will allow us to observe in a later step that we really have two alternative paths, instead of those that appear to us.

The code after merge if logic is as follows:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character = text.charCodeAt(i) + shift;
    if (
      (text.charCodeAt(i) >= LETTER.a &&
        text.charCodeAt(i) <= LETTER.z &&
        text.charCodeAt(i) + shift > LETTER.z) ||
      (text.charCodeAt(i) >= LETTER.A &&
        text.charCodeAt(i) <= LETTER.Z &&
        text.charCodeAt(i) + shift > LETTER.Z)
    ) {
      character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
    }
    if (
      (text.charCodeAt(i) >= LETTER.a &&
        text.charCodeAt(i) <= LETTER.z &&
        text.charCodeAt(i) + shift > LETTER.z &&
        !(text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z)) ||
      (text.charCodeAt(i) >= LETTER.A &&
        text.charCodeAt(i) <= LETTER.Z &&
        !(text.charCodeAt(i) + shift > LETTER.Z))
    ) {
      character = text.charCodeAt(i) + shift;
    }

    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character = text.charCodeAt(i) - shift;
    if (
      (text.charCodeAt(i) >= LETTER.a &&
        text.charCodeAt(i) <= LETTER.z &&
        text.charCodeAt(i) - shift < LETTER.a) ||
      (text.charCodeAt(i) >= LETTER.A &&
        text.charCodeAt(i) <= LETTER.Z &&
        text.charCodeAt(i) - shift < LETTER.A)
    ) {
      character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
    }
    if (
      (text.charCodeAt(i) >= LETTER.a &&
        text.charCodeAt(i) <= LETTER.z &&
        !(text.charCodeAt(i) - shift < LETTER.a)) ||
      (text.charCodeAt(i) >= LETTER.A &&
        text.charCodeAt(i) <= LETTER.Z &&
        !(text.charCodeAt(i) - shift < LETTER.A))
    ) {
      character = text.charCodeAt(i) - shift;
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Step 5. Simplifying the logic of the algorithm

In this step, we have to reason that our algorithm does not need two if control structures. Rather, both the cipher and the decipher functions have an if-else control structure. Focusing on the function cipher it is observed that there are two possible options for assigning the value to the variable character. The first possibility is the one obtained from the corresponding first if.

character = text.charCodeAt(i) + shift - NUMBER_LETTERS;

The second possible value that is obtained both in the default case and the one obtained from the other control structure if is the following ones:

character = text.charCodeAt(i) + shift;

Therefore, it is possible to remove the logic of the second if and transform the control structure into else corresponding to the first control structure if since, in the case that the condition of this if is not met, the second possible value will be assigned for the variable character. Whether the second if is fulfilled or not when assigned by the default value.

The code after of this refactoring is as follows:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character;
    if (
      (text.charCodeAt(i) >= LETTER.a &&
        text.charCodeAt(i) <= LETTER.z &&
        text.charCodeAt(i) + shift > LETTER.z) ||
      (text.charCodeAt(i) >= LETTER.A &&
        text.charCodeAt(i) <= LETTER.Z &&
        text.charCodeAt(i) + shift > LETTER.Z)
    ) {
      character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
    } else {
      character = text.charCodeAt(i) + shift;
    }

    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    if (
      (text.charCodeAt(i) >= LETTER.a &&
        text.charCodeAt(i) <= LETTER.z &&
        text.charCodeAt(i) - shift < LETTER.a) ||
      (text.charCodeAt(i) >= LETTER.A &&
        text.charCodeAt(i) <= LETTER.Z &&
        text.charCodeAt(i) - shift < LETTER.A)
    ) {
      character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
    } else {
      character = text.charCodeAt(i) - shift;
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Step 6. Encapsulate conditionals

The condition of our algorithm is quite complex and difficult to understand because it lacks semantic value. Therefore, the next step in the code is known as encapsulate conditionals.

Specifically, we focus on encapsulating the cipher and decipher conditions:

cipher:

(text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z && text.charCodeAt(i) + shift > LETTER.z) 
||
(text.charCodeAt(i) >= LETTER.A && text.charCodeAt(i) <= LETTER.Z && text.charCodeAt(i) + shift > LETTER.Z)

decipher:

(text.charCodeAt(i) >= LETTER.a && text.charCodeAt(i) <= LETTER.z && text.charCodeAt(i) - shift < LETTER.a) 
||
(text.charCodeAt(i) >= LETTER.A && text.charCodeAt(i) <= LETTER.Z && text.charCodeAt(i) - shift < LETTER.A)

In fact, this logic can be summarized in the following four functions:

function isOutLowerCharacterCipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.a &&
    text.charCodeAt(position) <= LETTER.z &&
    text.charCodeAt(position) + shift > LETTER.z
  );
}
function isOutUpperCharacterCipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.A &&
    text.charCodeAt(position) <= LETTER.Z &&
    text.charCodeAt(position) + shift > LETTER.Z
  );
}

function isOutLowerCharacterDecipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.a &&
    text.charCodeAt(position) <= LETTER.z &&
    text.charCodeAt(position) - shift < LETTER.a
  );
}

function isOutUpperCharacterDecipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.A &&
    text.charCodeAt(position) <= LETTER.Z &&
    text.charCodeAt(position) - shift < LETTER.A
  );
}

The code after performing this encapsulation is as follows:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    let character;
    if (
      isOutLowerCharacterCipher(text, i, shift) ||
      isOutUpperCharacterCipher(text, i, shift)
    ) {
      character = text.charCodeAt(i) + shift - NUMBER_LETTERS;
    } else {
      character = text.charCodeAt(i) + shift;
    }

    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    if (
      isOutLowerCharacterDecipher(text, i, shift) ||
      isOutUpperCharacterDecipher(text, i, shift)
    ) {
      character = text.charCodeAt(i) - shift + NUMBER_LETTERS;
    } else {
      character = text.charCodeAt(i) - shift;
    }
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Step 7. Remove if-else structure control

The control structure if-else makes an assignment on the same variable (character). Therefore, you can extract the conditional logic from the if and store it in a variable as follows:

 const isOutAlphabet =
      isOutLowerCharacterCipher(text, i, shift) ||
      isOutUpperCharacterCipher(text, i, shift);

The assignment to the variable character is only modified by a rotation value that can have two possible values:

NUMBER_LETTERS
0 (NO_ROTATION);

Therefore, we can define the variable rotation so that it allows us to raise a level of granularity in the code as follows:

const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;

The resulting code is as follows:

const isOutAlphabet =
isOutLowerCharacterCipher(text, i, shift) ||
isOutUpperCharacterCipher(text, i, shift);
const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
const character = text.charCodeAt(i) + shift - rotation;

cipher = cipher.concat(String.fromCharCode(character));

The code of the two resulting functions after this step is as follows:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    const isOutAlphabet =
      isOutLowerCharacterCipher(text, i, shift) ||
      isOutUpperCharacterCipher(text, i, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(i) + shift - rotation;

    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let i = 0; i < text.length; i++) {
    const isOutAlphabet =
      isOutLowerCharacterDecipher(text, i, shift) ||
      isOutUpperCharacterDecipher(text, i, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(i) - shift + rotation;
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Step 8. Variable Naming

The last step to finish refactoring our algorithm is rename the variable i in loops to a more suitable name such as position (This change may seem "small" but it is very important that we assign semantic value to the variables, including the classic i, j and k in the loops.

The final result of our algorithm, after applying these simple steps are as follows:

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let position = 0; position < text.length; position++) {
    const isOutAlphabet =
      isOutLowerCharacterCipher(text, position, shift) ||
      isOutUpperCharacterCipher(text, position, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(position) + shift - rotation;

    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let position = 0; position < text.length; position++) {
    const isOutAlphabet =
      isOutLowerCharacterDecipher(text, position, shift) ||
      isOutUpperCharacterDecipher(text, position, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(position) - shift + rotation;
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

Conclusions

In this post, we have presented some recommendations for do a refactoring from a basic solution to a comprehensible code.

In this post, I have shown you a step by step of my reasoning. There are other ways, of course, and some decisions may not be the most appropriate on your point of view. For all these reasons, I invite you to share your thoughts with the entire community whenever it is from a constructive point of view.

This challenge is intended to make all colleagues in the industry who think this is difficult for them. So, they can see how other colleagues perform refactoring tasks step by step.

In the next post, related to this challenge, I will continue to evolve the code by trying to give a vision of the solution from a functional programming point of view.

Finally, the points we have addressed are the following ones:

Magic numbers
Extract similar code from if-else
Avoid else
Merge the IF logic
Simplifying the logic of the algorithm
Encapsulate conditionals
Remove if-else structure control
Variable Naming

Ahh, of course, I leave you the codes, both the original and the final, so you can make a final balance of it.

function cipher(text, shift) {
  var cipher = '';
  shift = shift % 26;
  for (var i = 0; i < text.length; i++) {
    if (text.charCodeAt(i) >= 65 && text.charCodeAt(i) <= 90) {
      if (text.charCodeAt(i) + shift > 90) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) + shift - 26),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
      }
    } else if (text.charCodeAt(i) >= 97 && text.charCodeAt(i) <= 122) {
      if (text.charCodeAt(i) + shift > 122) {
        cipher = cipher.concat(
          String.fromCharCode(text.charCodeAt(i) + shift - 26),
        );
      } else {
        cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
      }
    } else {
      // blank space
      cipher = cipher.concat(String.fromCharCode(text.charCodeAt(i) + shift));
    }
  }
  return cipher.toString();
}

function decipher(text, shift) {
  var decipher = '';
  shift = shift % 26;
  for (var i = 0; i < text.length; i++) {
    if (text.charCodeAt(i) >= 65 && text.charCodeAt(i) <= 90) {
      if (text.charCodeAt(i) - shift < 65) {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift + 26),
        );
      } else {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift),
        );
      }
    } else if (text.charCodeAt(i) >= 97 && text.charCodeAt(i) <= 122) {
      if (text.charCodeAt(i) - shift < 97) {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift + 26),
        );
      } else {
        decipher = decipher.concat(
          String.fromCharCode(text.charCodeAt(i) - shift),
        );
      }
    } else {
      // blank space
      decipher = decipher.concat(
        String.fromCharCode(text.charCodeAt(i) - shift),
      );
    }
  }
  return decipher.toString();
}

console.assert(
  cipher('Hello World', 1) === 'Ifmmp!Xpsme',
  `${cipher('Hello World', 1)} === 'Ifmmp!Xpsme'`,
);
console.assert(
  decipher(cipher('Hello World', 3), 3) === 'Hello World',
  `${decipher(cipher('Hello World', 3), 3)} === 'Hello World'`,
);

And the final code is the following ones:

const NUMBER_LETTERS = 26;
const NO_ROTATION = 0;
const LETTER = {
  a: 65,
  z: 90,
  A: 97,
  Z: 122,
};

function isOutLowerCharacterCipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.a &&
    text.charCodeAt(position) <= LETTER.z &&
    text.charCodeAt(position) + shift > LETTER.z
  );
}
function isOutUpperCharacterCipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.A &&
    text.charCodeAt(position) <= LETTER.Z &&
    text.charCodeAt(position) + shift > LETTER.Z
  );
}

function isOutLowerCharacterDecipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.a &&
    text.charCodeAt(position) <= LETTER.z &&
    text.charCodeAt(position) - shift < LETTER.a
  );
}

function isOutUpperCharacterDecipher(text, position, shift) {
  return (
    text.charCodeAt(position) >= LETTER.A &&
    text.charCodeAt(position) <= LETTER.Z &&
    text.charCodeAt(position) - shift < LETTER.A
  );
}

function cipher(text, shift) {
  let cipher = '';
  shift = shift % NUMBER_LETTERS;
  for (let position = 0; position < text.length; position++) {
    const isOutAlphabet =
      isOutLowerCharacterCipher(text, position, shift) ||
      isOutUpperCharacterCipher(text, position, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(position) + shift - rotation;

    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

function decipher(text, shift) {
  let cipher = '';

  shift = shift % NUMBER_LETTERS;
  for (let position = 0; position < text.length; position++) {
    const isOutAlphabet =
      isOutLowerCharacterDecipher(text, position, shift) ||
      isOutUpperCharacterDecipher(text, position, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(position) - shift + rotation;
    cipher = cipher.concat(String.fromCharCode(character));
  }
  return cipher.toString();
}

console.assert(
  cipher('Hello World', 1) === 'Ifmmp!Xpsme',
  `${cipher('Hello World', 1)} === 'Ifmmp!Xpsme'`,
);
console.assert(
  decipher(cipher('Hello World', 3), 3) === 'Hello World',
  `${decipher(cipher('Hello World', 3), 3)} === 'Hello World'`,
);

Clean Code Applied to JavaScript - Part VII: Practical Refactoring Example: Ceaser Cipher