DEV Community

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

lukaszahradnik profile image
Lukáš Zahradník • Edited on

Nice refactor.

I would refactor it even more because cipher and decipher has the same logic, just shift and rotation are inversed (notice modifier in function cipherLogic). Also this means, that only two functions for checking alphabet are needed instead of four (cipher and deciper uses same functions).

function cipher(text, shift) {
  return cipherLogic(text, shift, false)

function decipher(text, shift) {
  return cipherLogic(text, shift, true)

function cipherLogic(text, shift, decipher) {
  const modifier = decipher ? -1 : 1; 
  let cipher = '';
  shift = modifier * (shift % NUMBER_LETTERS);

  for (let position = 0; position < text.length; position++) {
    const isOutAlphabet =
      isOutLowerCharacter(text, position, shift) ||
      isOutUpperCharacter(text, position, shift);
    const rotation = isOutAlphabet ? NUMBER_LETTERS : NO_ROTATION;
    const character = text.charCodeAt(position) + shift + modifier * rotation;

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

It's not really good practice to mutate function parameters (shift) and I would also store in a variable the result of text.charCodeAt(position) in alphabet checks.