JavaScript - Caesar Cipher

Question

I know there have been a few posts about Caesar Ciphers in the past, which I have had a look at, but I haven't found an answer which has helped me to solve this kata, hence my post.

The language is JavaScript. I've written 3 tests, 2 of which are passing so far, but the third is not. I tried to use a nested for loop to loop over the alphabet and the str, and compare them, then shift the alphabet index up/down according to whatever the number was, then push that letter into a new array, and return the joined array at the end.

It's working for positive numbers, but not negatives. (I should also point out that I haven't thought of how to handle spaces yet, I just wanted to get it working for single words first, then take it from there, thanks!)

Any pointers in the right direction would be appreciated.

Kata Instructions:

The function caesarCipher should take a string and a number (n) and return a new string with a Caesar cipher applied. A Caesar cipher replaces each plaintext letter with a different one a fixed number of places up or down the alphabet. N represents the number of shifts up or down the alphabet should be applied. It may be negative or positive.

  E.g.
  caesarCipher('hello', 2)
    --> 'jgnnq'
  caesarCipher('hello world!', -3)
    --> 'ebiil tloia!'

My tests:

const caesarCipher = require("../katas/caesar-cipher");
const { expect } = require("chai");

describe.only("caesarCipher", () => {
  it("returns an empty string when passed an empty string", () => {
    const alphabet = [
      "a",
      "b",
      "c",
      "d",
      "e",
      "f",
      "g",
      "h",
      "i",
      "j",
      "k",
      "l",
      "m",
      "n",
      "o",
      "p",
      "q",
      "r",
      "s",
      "t",
      "u",
      "v",
      "w",
      "x",
      "y",
      "z"
    ];
    const str = "";
    const num = 2;
    const actualResults = caesarCipher(alphabet, str, num);
    const expectedResults = "";
    expect(actualResults).to.equal(expectedResults);
  });
  it("returns a string with the letters replaced by the number of shifts up the alphabet", () => {
    const alphabet = [
      "a",
      "b",
      "c",
      "d",
      "e",
      "f",
      "g",
      "h",
      "i",
      "j",
      "k",
      "l",
      "m",
      "n",
      "o",
      "p",
      "q",
      "r",
      "s",
      "t",
      "u",
      "v",
      "w",
      "x",
      "y",
      "z"
    ];
    const str = "hi";
    const num = 2;
    const actualResults = caesarCipher(alphabet, str, num);
    const expectedResults = "jk";
    expect(actualResults).to.equal(expectedResults);
  });
  it("returns a string with the letters replaced by the number of shifts down the alphabet", () => {
    const alphabet = [
      "a",
      "b",
      "c",
      "d",
      "e",
      "f",
      "g",
      "h",
      "i",
      "j",
      "k",
      "l",
      "m",
      "n",
      "o",
      "p",
      "q",
      "r",
      "s",
      "t",
      "u",
      "v",
      "w",
      "x",
      "y",
      "z"
    ];
    const str = "dog";
    const num = -3;
    const actualResults = caesarCipher(alphabet, str, num);
    const expectedResults = "ald";
    expect(actualResults).to.equal(expectedResults);
  });
});

My Solution:

function caesarCipher(alphabet, str, num) {
  const strToArray = str.split("");
  console.log(strToArray);
  const cipheredStr = [];
  for (let i = 0; i < strToArray.length; i++) {
    for (let j = 0; j < alphabet.length; j++) {
      if (strToArray[i] === alphabet[j] && Math.sign(num) === 1) {
        console.log(Math.sign(num));
        cipheredStr.push(alphabet[(j += num)]);
      } else if (strToArray[i] === alphabet[j] && Math.sign(num) === -1) {
        console.log(Math.sign(num));
        console.log(alphabet[(j -= num)]);
        cipheredStr.push(alphabet[(j -= num)]);
      }
    }
  }
  console.log(cipheredStr.join(""));
  return cipheredStr.join("");
}

The Results:

caesarCipher
[]

    ✓ returns an empty string when passed an empty string
[ 'h', 'i' ]
1
1
jk
    ✓ returns a string with the letters replaced by the number of shifts up the alphabet
[ 'd', 'o', 'g' ]
-1
g
-1
r
-1
j
jum
    1) returns a string with the letters replaced by the number of shifts down the alphabet


  2 passing (15ms)
  1 failing

  1) caesarCipher
       returns a string with the letters replaced by the number of shifts down the alphabet:

      AssertionError: expected 'jum' to equal 'ald'
      + expected - actual

      -jum
      +ald
      
      at Context.<anonymous> (spec/caesar-cipher.spec.js:108:30)
      at processImmediate (internal/timers.js:456:21)

Hi @IndigoDreams, please, provide code source/snippets instead of images. — mgarcia, Feb 16 '20 at 17:26
The solution could be way simpler. I read the algorithm in wiki: charOutIndex = (charInIndex + num) % alphabet.length — bitWorking, Feb 16 '20 at 17:53
I've used the modulo operator before, but wouldn't have though to use it in this situation. I'll take a look into it, thanks :) — IndigoDreams, Feb 16 '20 at 18:05
*"It's working for positive numbers, but not negatives"* are you aware that you can write every negative index as a positive one? for example: `-3` is the same rotation as `+23` — Thomas, Feb 16 '20 at 18:40
This fixes the JavaScript modulo bug which gives you negative result: https://web.archive.org/web/20090717035140if_/javascript.about.com/od/problemsolving/a/modulobug.htm — bitWorking, Feb 16 '20 at 20:36

score 1 · Answer 1 · answered Feb 16 '20 at 17:53

The problem is that when num is negative, you are doing the mapping:

cipheredStr.push(alphabet[(j -= num)]);

But num is negative. When you subtract a negative number what you are doing is adding its absolute value.

Instead, you should do:

cipheredStr.push(alphabet[j + num]);

Also, note that to compute the index in the alphabet you don't need to put the = in there.

Side notes

I understand that your solution is a work in progress. You have to take into consideration:

What happens when you sum j + num to do the translation and it goes out of boundaries of your alphabet. Same thing could happen with a negative value of num.
In the declaration of the problem, it states that caesarCipher should only accept two parameters, but you are passing the alphabet as first parameter!

Good luck with your code and keep trying :)

Ok, thanks, yes that's another thing that I've started to think about but haven't addressed yet. I was thinking i'd maybe just carry on looping back round the alphabet, so if the num took the letter beyond Z, then it'd just loop back round through A and vice versa, but haven't quite got that far yet. Thanks for the suggestions everyone! :) — IndigoDreams, Feb 16 '20 at 18:02

score 0 · Answer 2 · answered Jun 07 '22 at 06:10

let alphabets = 'abcdefghijklmnopqrstuvwxyz';
let arr = alphabets.split('')
// Cipher Get
function getCipher(str= alphabets, shift=3){
    return arr.reduce((a, c, i) => {
        let result = [...a]
        let tIndex = ( i + shift) % arr.length
        result[i]=arr[tIndex]
        return result;
    },[])
}

// Encrypt 
let stringToEnc = 'danger'
console.log('Plain Text -', stringToEnc)
let cipheredAlphabets = getCipher()
let encryptedStr = stringToEnc
    .toLowerCase()
    .split('')
    .map(function(p, i){
        let indexInAlphabets = arr.findIndex(c => p == c)
        return (cipheredAlphabets[indexInAlphabets])
    })
let encryptedText = encryptedStr.join('')
console.log('encrypted text - ', encryptedText)

// Decrypt
let cipherForDecrypt = getCipher(alphabets, -3)

let decryptedStr = encryptedText
    .toLowerCase()
    .split('')
    .map(function(p, i){
        let indexInAlphabets = cipheredAlphabets.findIndex(c => p == c)
        return (arr[indexInAlphabets])
    })
console.log('decrypted text - ', decryptedStr.join(''))

JavaScript - Caesar Cipher

2 Answers2