Hi! Just finished writing the code for the challenge.
Would be glad if you gave me some advice on refactoring it.
function rot13(str) {
const alphabet = " ABCDEFGHIJKLMNOPQRSTUVWXYZ";
const alphS = alphabet.split('');
const regex = new RegExp(/[^a-zA-Z]+/g);
// transform string to array
let arr = str.split('');
let newArr = [];
for (let i = 0; i < arr.length; i++){
//go to index of a letter + 13 (end of alphabet)
let newIndex = alphS.indexOf(arr[i]) + 13;
//push spaces
if (arr[i] === ' ') {
newArr.push(' ')
continue;
}
//push special characters
else if(regex.test(arr[i])){
newArr.push(arr[i]);
continue;
}
else if (newIndex >= 27){
let newIndex = alphS.indexOf(arr[i]) - 13;
newArr.push(alphS[newIndex]);
continue;
}
//push letter to new array
newArr.push(alphS[newIndex]);
}
return newArr.join('');
}
console.log(rot13("SERR CVMMN!"));
const regex = new RegExp(/[^a-zA-Z]+/g);
Your regex here filters for everything that’s not an uppercase or a lowercase letter. But did you check out what the output is if you actually use the function on a lowercase letter?
You have //push spaces
and then //push special characters
Maybe one of these is unnessessary?
Thank you!
I removed the redundant code for pushing whitespaces, since my regex already affects them.
I don’t understand though why the regex works that way when I pass a lowercase phrase… It’s supposed to ignore letters no matter the case, apparently it does not.
The regex ignores lowercase letters, that works
but alphS has no lowercase letters inside, so here
always results into newIndex = -1 + 13
and finally the last line of the for loop newArr.push(alphS[newIndex]); is the same as newArr.push(alphS[12]); for every lowercase letter.
Helps that with completing the function for lowercase letters?
It’s an interesting approach indeed. As far as I can see the implementation is pretty much the same, but with different methods.
I tried to write some pseudo-code for it:
check charcodes with regex [^a-zA-Z]+
if is letter (we supply this with a certain ASCII index range(either uppercase or lowercase)) => for elements of string do str.charCodeAt(i);
2.1) every letter element +13, mind end of the alphabet;
2.2) add the new indexes to method str.fromCharCode(ind1, ind2,…)
if not letter add to the method (str.fromCharCode(ind1, ind2,…) straightaway.
Now the issue is I don’t know how to add all deciphered ASCII indexes into the method at the same time. Meaning we call the method only once, not every time a new index is created.
For the particular task (only uppercase letters) you’re being right, we don’t need “a-z” here. Although if you’re going to implement case insensitiveness in this code (which wasn’t the issue at first ), you would need to consider both cases.
You are given a string of all capital characters I believe? So no regex is needed.
Look at the ascii chart. All capitals are contained within a certain block of numbers…
And that is all I am going to say without spoiling this method.
Right, of course we don’t need regex here, we already have all we need.
I think I figured it out.
We create an array of new indexes and then simply spread them, e.g. String.fromCharCode(…arrOfNewIndexes)
@hbar1st My idea didn’t work. I’m logging off now. Thank you for your time and advice!
It’s really interesting how the task can be accomplished without having to create an array.
@hbar1st@colinthornton
Well that was certainly an interesting task and I found out some new things for myself.
I hope it looks at least a bit like what you had in mind.
And I’m sure the “if” statements can be optimized, I’m just not sure how : /
function rot13(str) {
let endStr = '';
for (const value of str) {
if (91 > str.charCodeAt(str.indexOf(value)) && str.charCodeAt(str.indexOf(value)) > 64) {
if (13 + str.charCodeAt(str.indexOf(value)) >= 91) {
endStr += String.fromCharCode(str.charCodeAt(str.indexOf(value)) - 13);
}
else {
endStr += String.fromCharCode(13 + str.charCodeAt(str.indexOf(value)))
}
}
else {
endStr += String.fromCharCode(str.charCodeAt(str.indexOf(value)))
}
}
return endStr;
}
console.log(rot13("SERR CVMMN!"));
Well, the “% 26) + 65” part I had to google up, did not manage to understand it by myself. Nevertheless it’s a funny way of getting rid of them “if” cycles in this case.
function rot13(str) {
let endStr = '';
for (const value of str) {
if (91 > str.charCodeAt(str.indexOf(value)) && str.charCodeAt(str.indexOf(value)) > 64) {
endStr += String.fromCharCode((str.charCodeAt(str.indexOf(value)) % 26) + 65);
}
else {
endStr += String.fromCharCode(str.charCodeAt(str.indexOf(value)))
}
}
return endStr;
couple more things at least.
1- str.charCodeAt(str.indexOf(value)) is repeated a lot, how about putting it in a variable?
2- the else statement 's endStr statement is complicated.
(you are transferring the blanks to their ascii and then back? why not just concatenate them as they are?)
3- we know the input is capital letters or blanks.
So maybe the if statement can be made simpler?
Thank you!!
), you would need to consider both cases.
