Caesars Cipher Easier Solution?

The solutions given in the guide were to difficult to understand for me. So here is an easier solution:

function rot13(str) {

  let alphabetArr = "ABCDEFGHIJKLMNOPQRSTUVWXYZ".split("");
  let converted = "";

  for (let i = 0; i < str.length; i++) {
    
    if (alphabetArr.indexOf(str[i]) === -1) {
        converted += str[i];
    } 
      else if (alphabetArr.indexOf(str[i]) < 13) {
        converted += alphabetArr[(alphabetArr.indexOf(str[i])) + 13];

    } else if (alphabetArr.indexOf(str[i]) >= 13) {
        converted += alphabetArr[alphabetArr.indexOf(str[i]) - 13];
    }

}

return converted;

};
rot13("SERR CVMMN!");

Let me know if you find it easier or not? Also any idea, how I can incorporate forEach() & map() in this?

Hi @FST360!

Welcome to the forum!

When posting full working solutions please wrap them in [spoiler] tags for those who haven’t worked on the challenge yet.

3 Likes

With regards to your solution, yes, you could argue that it is more readable - and that is not without merit. I think the counter argument would be that the other solutions are also readable, once you are more familiar with those patterns.

If I were to critique your solution, once thing I would say is that the last else if can just be and else - there are no conditions under which that last if condition will be false. But that’s a trivial thing.

Also, do you need to split the alphabetArray? Those methods work on strings, too.

From an efficiency standpoint, I would be worried about how you’re progressively concatenating the string. For each iteration, you are creating a whole new string and garbage collecting the old string. It’s not like with a number where you’re just adding a number but it’s still in the same memory location. This is a minor thing if the coded message is small, but it is something about which to think. This wouldn’t be an issue in some languages where a string is an array of characters, but JS doesn’t think that way.

If you want to use map or forEach, you would have to convert it to an array first - those work on arrays. But that is a good solution. If you want to try that, I think that would be worthwhile. I think map is a better solution. forEach could work but I think would be a little less elegant. So, you would need to separate the string into an array, map it to change the values, then convert the array back to a string.

1 Like

If you’re curious, this would be a simple map implementation. Don’t look if you want to try it yourself. (recommended)

const alphabetArr = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

const rot13 = str => {
  return str.split('').map((char) => {
    if (alphabetArr.indexOf(char) === -1) {
      return char;
    } else if (alphabetArr.indexOf(char) < 13) {
      return alphabetArr[alphabetArr.indexOf(char) + 13];
    } else {
      return alphabetArr[alphabetArr.indexOf(char) - 13];
    }
  }).join('');
};

console.log(rot13("SERR CVMMN!"));

This is a slightly more readable approach, breaking things apart:

const alphabetArr = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";

const caeserifyChar = char => {
  if (alphabetArr.indexOf(char) === -1) {
    return char;
  } else if (alphabetArr.indexOf(char) < 13) {
    return alphabetArr[alphabetArr.indexOf(char) + 13];
  } else {
    return alphabetArr[alphabetArr.indexOf(char) - 13];
  }
}

const rot13 = str => str.split('').map(caeserifyChar).join('');

console.log(rot13("SERR CVMMN!"));

I probably would have done the character logic a little differently (just done a little math on the character codes), but that’s the basic idea.

:grimacing:, sorry about that.

Thank you for the critique, really needed to learn about these things.

Were you interested in contributing this to the guide? or were you just asking for feedback?

Or both?

If you are interested in contributing this to the guide then follow these suggestions.

For future contributions, please wrap your solution within :

[details]
```
code goes here...
```
[/details]

Also, provide all of the necessary code to pass the challenge and the challenge link.