Kevin Powell - Community•15mo ago

For loop in JavaScript

Hello there, I am creating a simple webpage for a Caesar Cipher. I wrote the Cipher in Object-Oriented. Something is wrong with a method though. Im not exactly sure but when I shift anything other than A, the encryption is wrong.

encrypt() {
      for (let i = 0; i < this.text_length; i++) {
        let char = this.plain_text[i].toUpperCase();
        if (char === " ") {
          this.encrypted_string += char;
        } else {
          let location = this.alphabet.indexOf(char);
          let new_location = (location + this.shift) % 26;
          this.encrypted_string += this.alphabet[new_location];
        }
      }
      return this.encrypted_string;
    }

encrypt() {
      for (let i = 0; i < this.text_length; i++) {
        let char = this.plain_text[i].toUpperCase();
        if (char === " ") {
          this.encrypted_string += char;
        } else {
          let location = this.alphabet.indexOf(char);
          let new_location = (location + this.shift) % 26;
          this.encrypted_string += this.alphabet[new_location];
        }
      }
      return this.encrypted_string;
    }

As you can see in the image. B plus 1 shouldnt be L

17 Replies

13eck•15mo ago

You're calling this.plain_text,this.alphabet, and this.shift without showing what those are so we can't really help much. Can you provide the whole class/function that you're using?

jj_roby_1993•15mo ago

class Cipher {
  constructor(shift=0, plain_text="", encrypted_string="") {
    this.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    this.shift = shift;
    this.plain_text = plain_text;
    this.text_length = 0;
    this.encrypted_string = encrypted_string;
  }

  setShift(shiftValue) {
    this.shift = shiftValue;
  }

  setPlainText(plainText) {
    this.plain_text = plainText;
  }

  setTextLength() {
    this.text_length = this.plain_text.length;
  }

  encrypt() {
    for (let i = 0; i < this.text_length; i++) {
      let char = this.plain_text[i].toUpperCase();
      if (char === " ") {
        this.encrypted_string += char;
        continue;
      }
      let location = this.alphabet.indexOf(char);
      let new_location = (location + this.shift) % 26;
      this.encrypted_string += this.alphabet[new_location];
    }
    return this.encrypted_string; 
  }
}

const shift = document.getElementById("shiftInput")
const plaintext = document.getElementById("plainTextArea")
const encryptBtn = document.getElementById("encryptBtn")
const encryptedArea = document.getElementById("encryptedArea")
const clearBtn = document.getElementById("clearBtn")

const cipherObj01 = new Cipher();

encryptBtn.addEventListener('click', () => {
  cipherObj01.setShift(shift.value);
  cipherObj01.setPlainText(plaintext.value);
  cipherObj01.setTextLength();
  const encryptionString = cipherObj01.encrypt()
  encryptedArea.value = encryptionString;
});

clearBtn.addEventListener('click', () => {
  window.location.reload()
});

class Cipher {
  constructor(shift=0, plain_text="", encrypted_string="") {
    this.alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
    this.shift = shift;
    this.plain_text = plain_text;
    this.text_length = 0;
    this.encrypted_string = encrypted_string;
  }

  setShift(shiftValue) {
    this.shift = shiftValue;
  }

  setPlainText(plainText) {
    this.plain_text = plainText;
  }

  setTextLength() {
    this.text_length = this.plain_text.length;
  }

  encrypt() {
    for (let i = 0; i < this.text_length; i++) {
      let char = this.plain_text[i].toUpperCase();
      if (char === " ") {
        this.encrypted_string += char;
        continue;
      }
      let location = this.alphabet.indexOf(char);
      let new_location = (location + this.shift) % 26;
      this.encrypted_string += this.alphabet[new_location];
    }
    return this.encrypted_string; 
  }
}

const shift = document.getElementById("shiftInput")
const plaintext = document.getElementById("plainTextArea")
const encryptBtn = document.getElementById("encryptBtn")
const encryptedArea = document.getElementById("encryptedArea")
const clearBtn = document.getElementById("clearBtn")

const cipherObj01 = new Cipher();

encryptBtn.addEventListener('click', () => {
  cipherObj01.setShift(shift.value);
  cipherObj01.setPlainText(plaintext.value);
  cipherObj01.setTextLength();
  const encryptionString = cipherObj01.encrypt()
  encryptedArea.value = encryptionString;
});

clearBtn.addEventListener('click', () => {
  window.location.reload()
});

13eck•15mo ago

Off the top of my head, I'm guessing the issue is with you setting the shift value. All inputs in HTML are read as strings by JS, so even if the HTML says the shift value is a number input with a value of 4 JS reads it as the string "4". And that can cause problems if you're trying to add a string (as that will concatenate the values instead of adding them).

jj_roby_1993•15mo ago

ahh 🤔 i didnt get any errors tho @cvanilla13eck how do you cast from string to int in js?

13eck•15mo ago

parseInt({str}, 10) (10 being the radix; you'd use 2 for binary and 16 for hex)

setShift(shiftValue) {
    this.shift = parseInt(shiftValue, 10);
  }

setShift(shiftValue) {
    this.shift = parseInt(shiftValue, 10);
  }

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/parseInt

jj_roby_1993•15mo ago

Excellent! it works

13eck•15mo ago

WOO!

jj_roby_1993•15mo ago

😆 thanks so much

13eck•15mo ago

Just because I could, here's another way of doing the encrypt function by turning the string into an array and using the .map() functionality to modify the letter, then using .join() to make it into a string again:

const encrypt = (str) => {
  const alphabet= "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  const alphLength = alphabet.length;
  return str.split("").map((letter, idx) => {
    const curLetterIdx = alphabet.indexOf(letter.toUpperCase());
    if (curLetterIdx < 0) return letter;
    const newIdx = (curLetterIdx + idx + 1) % alphLength;
    return alphabet[newIdx];
    }).join("");
}

const encrypt = (str) => {
  const alphabet= "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  const alphLength = alphabet.length;
  return str.split("").map((letter, idx) => {
    const curLetterIdx = alphabet.indexOf(letter.toUpperCase());
    if (curLetterIdx < 0) return letter;
    const newIdx = (curLetterIdx + idx + 1) % alphLength;
    return alphabet[newIdx];
    }).join("");
}

This way you don't need to store the text length and don't need to do a for loop

jj_roby_1993•15mo ago

ahh i see awesome

13eck•15mo ago

I'm not a fan of the old for loop, so I prefer to use forEach(), .map(), and .filter() as it handles the minutia of index tracking

jj_roby_1993•15mo ago

yeah, im new to js im comming from python js has a shit ton of methods

13eck•15mo ago

If you prefer a more imperative-style, here's one using the for...of loop:

const encrypt = (str) => {
  // this is the alphabet being used
  const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  // the length so we don't have a "magic number" below
  const alphLength = alphabet.length;
  // create an empty string to store the returned value
  let retStr = "";
  // initialize the index
  let idx = 1;
  // `for...of` will turn the string into an array
  // and loop over it one at a time
  for (const letter of str) {
    // the `.indexOf()` returns the index number (or -1 if
    // the value doesn't exist)
    const curLetterIdx = alphabet.indexOf(letter.toUpperCase());
    // check if it's not in the alphabet
    if (curLetterIdx < 0) {
      // add the letter as-is to the return string
      retStr += letter;
      // increment the index
      idx++;
      // break out of this loop to not run the below code
      continue;
    }
    // get the index of the shifted letter
    const newIdx = (curLetterIdx + idx) % alphLength;
    // increment the index
    idx++;
    // append the shifted letter to the string
    retStr += alphabet[newIdx];
  }

  // once it's all done give back the new, encoded, string!
  return retStr;
}

const encrypt = (str) => {
  // this is the alphabet being used
  const alphabet = "ABCDEFGHIJKLMNOPQRSTUVWXYZ";
  // the length so we don't have a "magic number" below
  const alphLength = alphabet.length;
  // create an empty string to store the returned value
  let retStr = "";
  // initialize the index
  let idx = 1;
  // `for...of` will turn the string into an array
  // and loop over it one at a time
  for (const letter of str) {
    // the `.indexOf()` returns the index number (or -1 if
    // the value doesn't exist)
    const curLetterIdx = alphabet.indexOf(letter.toUpperCase());
    // check if it's not in the alphabet
    if (curLetterIdx < 0) {
      // add the letter as-is to the return string
      retStr += letter;
      // increment the index
      idx++;
      // break out of this loop to not run the below code
      continue;
    }
    // get the index of the shifted letter
    const newIdx = (curLetterIdx + idx) % alphLength;
    // increment the index
    idx++;
    // append the shifted letter to the string
    retStr += alphabet[newIdx];
  }

  // once it's all done give back the new, encoded, string!
  return retStr;
}

This snippet has more lines of code as you're manually doing what the .map() method does under the hood, but it helps new programmers to see how it works you have no idea 😜 @jj_roby_1993 added comments to the above code to help you understand what's happening

jj_roby_1993•15mo ago

@cvanilla13eck Yo for some reason when I try to take encrypted text and use the negative of the shift number, I get undefined as a result. Shouldn't it still work and is essentially adding a negative number to the location (or subtracting) then modulo 26?

13eck•15mo ago

If you're trying to reverse it you can't use the % operator alone as it'll still be a negative number. After you do the % operator you need to check if it's a negative number and if so, subtract that number from 26. That should get you the correct letter

jj_roby_1993•15mo ago

That worked, thanks for that. I have an other issue if you dont mind... I'm trying to set up media queries. I got it re-adjusted for a tablet, but when I do a phone size, height: 100vh; is not working properly.

13eck•15mo ago

Please make a new post for a new question

Gaming

Programming

For loop in JavaScript