Caesar Cipher solution: Review my code and give your feedback

Guys, I finally came up with this solution after spending two and a half days on it. And the fun fact is, it doesn’t work with just ROT13, but with any number and in both directions!

To shift in right direction use a positive number and for left direction use a negative one.

I’m so happy that I was able to solve this challenge without looking at the solution. The code seems a bit verbose though :wink:

Please review my code and give your feedback.

const caesar = function(str, num = 13) {
    let arr = str.split('');
    let shift = num % 26;
    let shiftPositive = Math.abs(shift);

    const unicodeArray = arr.map(item => item.match(/[a-zA-Z]+/g) ? item.charCodeAt() : item);

    const finalArray = unicodeArray.map(item => {
        if (typeof item === 'number' && shift === 0) { // no shifting
            return item;

        } else if (typeof item === 'number' && shift > 0) { // shifting right

            if (item > 64 && item < 91) { // for UpperCase items
                item = item + shiftPositive;

                if (item > 90 && item < 116) {
                    return item - 26;
                } else {
                    return item;
                }

            } else if (item > 96 && item < 123) { // for LowerCase items
                item = item + shiftPositive;

                if (item > 122 && item < 148) {
                    return item - 26;
                } else {
                    return item;
                }
            }


        } else if (typeof item === 'number' && shift < 0) { // shifting left

            if (item > 64 && item < 91) { // for UpperCase items
                item = item - shiftPositive;

                if (item > 39 && item < 65) {
                    return item + 26;
                } else {
                    return item;
                }

            } else if (item > 96 && item < 123) { // for LowerCase items
                item = item - shiftPositive;

                if (item > 71 && item < 97) {
                    return item + 26;
                } else {
                    return item;
                }
            }


        } else {
            return item;
        }
    });

    const messageArray = finalArray.map(item => {
        if (typeof item === 'number') {
            return item = String.fromCharCode(item);
        } else {
            return item;
        }
    });

    return messageArray.join('');
    
};

The most important thing is that you have a working solution!

Yes, it is a little wordy. Go back and challenge yourself to simplify. For example, you can easily convert this:

const messageArray = finalArray.map(item => {
        if (typeof item === 'number') {
            return item = String.fromCharCode(item);
        } else {
            return item;
        }
    });

return messageArray.join('');

into one line. Also, there is plenty of room to simplify the if/else statements you have in the function you are passing to unicodeArray.map().

Hi there,

great work so far!

Ideas:

  • move the magic number, e.g. 26, 90, 116 etc. into variables/constants, so readers know what this stuff means

Keep us posted!