1

I'm doing a caesar-cypher. Trying to replace all characters from a string to a certain character from the shifted alphabet.

Here is my code so far

   public static String caesarify(String str, int key){
    String alphabetNormal = shiftAlphabet(0);
    String alphabetShifted = shiftAlphabet(key);
    for (int i =0; i < str.length();i++){
        for (int c =0; c < alphabetNormal.length(); c++) {
            if (str.charAt(i) == alphabetNormal.charAt(c)) {
                char replacement = alphabetShifted.charAt(c);
                str.replace(str.charAt(i), replacement);
            }
        }
    }
    return str;
}

public static String shiftAlphabet(int shift) {
    int start =0;
    if (shift < 0) {
        start = (int) 'Z' + shift + 1;
    } else {
        start = 'A' + shift;
    }
    String result = "";
    char currChar = (char) start;
    for(; currChar <= 'Z'; ++currChar) {
        result = result + currChar;
    }
    if(result.length() < 26) {
        for(currChar = 'A'; result.length() < 26; ++currChar) {
            result = result + currChar;
        }
    }
    return result;
}

I don't know why the string for example "ILIKEDONUTS" doesn't change to "JMJLFEPOVUT" when it's caesarified.

ajb
  • 31,309
  • 3
  • 58
  • 84
Quetzy
  • 19
  • 1
  • 4
  • "cypher is a graph query language for Neo4j". Don't use the "cypher" tag for a general cipher problem. I've removed the tag. – ajb Jul 25 '17 at 04:50
  • In order to help you it would be helpful if we knew what the actual output *is* and not only what it's not. – Paul Kertscher Jul 25 '17 at 05:13

2 Answers2

1

Don't use replace(), or any replace method, to replace a character at a given index in a String. It doesn't work. You're hoping that

str.replace(str.charAt(i), replacement);

will replace the i'th character of str. As pointed out in the other (now deleted) answer, str.replace doesn't change str itself, so you'd need to write

str = str.replace(str.charAt(i), replacement);

But that doesn't work. The replace() method doesn't know what your index i is. It only knows what character to replace. And, as the javadoc for replace() says, it replaces all characters in the string. So suppose that str.charAt(i) is 'a', and you want to replace it with 'd'. This code would replace all a characters with d, including (1) those that you already replaced with a earlier in the loop, so that this will defeat the work you've already done; and (2) a characters that come after this one, which you want to replace with d, but this will fail because later in the loop you will see d and replace it with g.

So you can't use replace(). There are a number of ways to replace the i'th character of a string, including using substring():

str = str.substring(0, i) + replacement + str.substring(i + 1);

But there are better ways, if you are going to replace every character. One is to create a StringBuilder from str, use StringBuilder's setCharAt method to change characters at specified indexes, and then convert the StringBuilder back to a String at the end. You should be able to look at the javadoc to find out what methods to use.

More: After looking into this more, I see why it was returning all A's. This inner loop has an error:

    for (int c =0; c < alphabetNormal.length(); c++) {
        if (str.charAt(i) == alphabetNormal.charAt(c)) {
            char replacement = alphabetShifted.charAt(c);
            str.replace(str.charAt(i), replacement);
        }
    }

Suppose key is 1, and the current character is 'C'. Your inner loop will eventually find C in alphabetNormal; it finds the corresponding character in alphabetShifted, which is D, and replaces C with D.

But then it loops back. Since the next character in alphabetNormal is D, it now matches the new str.char(i), which is now D, and therefore changes it again, to E. Then it loops back, and ... you get the picture.

ajb
  • 31,309
  • 3
  • 58
  • 84
  • ajb thanks so much for your help, i haven't learned about StringBuilder yet, but i'll try to read more about it so i can understand more your answer. – Quetzy Jul 26 '17 at 01:27
0
replace below line 

str.replace(str.charAt(i), replacement);

With

  str= str.replace(str.charAt(i), replacement);

or you can make a String arr and then replace character in that. in the end create a new string from that array and return.

a better version of  caesarify():



public static String caesarify(String str, int key){
    String alphabetNormal = shiftAlphabet(0);
    String alphabetShifted = shiftAlphabet(key);
    //create a char array 
   char[] arr=str.toCharArray();
   //always try to create variable outside of loop  
  char replacement
    for (int i =0; i < arr.length();i++){
        for (int c =0; c < alphabetNormal.length(); c++) {

            if (arr[i] == alphabetNormal.charAt(c)) {
                replacement = alphabetShifted.charAt(c);
               //replace char on specific position in the array
               arr[i]= replacement;
            }
        }
    }
    //return arrray as String
    return new String(arr);
}
DhaRmvEEr siNgh
  • 1,918
  • 2
  • 13
  • 17