0

I'm doing some practice on HackerRank and I'm stuck on a problem. The problem is:

Julius Caesar protected his confidential information by encrypting it using a cipher. Caesar's cipher shifts each letter by a number of letters. If the shift takes you past the end of the alphabet, just rotate back to the front of the alphabet. In the case of a rotation by 3, w, x, y and z would map to z, a, b and c.

Original alphabet: abcdefghijklmnopqrstuvwxyz

Alphabet rotated +3: defghijklmnopqrstuvwxyzabc

Example

input: 11 middle-Outz 2 output: okffng-Qwvb

I pass this test case, but I don't pass the rest of the test cases and it won't tell me which ones. What am I missing in my logic?

public static String caesarCipher(String s, int k) {
        String newString = "";
    // Write your code here
        for (int i=0; i<s.length(); i++) {
            char currentChar = s.charAt(i);
            int currentAscii = currentChar;
            if (currentAscii <65|| currentAscii>122 || (currentAscii>90 && currentAscii < 97)) {
                newString += currentChar;
            } else {
                currentAscii = currentChar+k;
                if (currentAscii > 122) {
                    currentAscii = Math.abs(currentAscii-26);
                } 
                newString += (char) currentAscii;
            }
        }

        return newString;
    }

.

  • Are you aware of idea behind `if (currentAscii <65|| currentAscii>122)`? Have you seen *all* characters which are between `65` and `122`? – Pshemo Sep 25 '21 at 17:55
  • Yes so I put that since that's where the alphabets are (between 65 and 122) but just realized there are some special characters in between them as well. I edited that in my code but still doesn't fix the issue –  Sep 25 '21 at 18:00
  • Lets start improving your code with (1) [not using "magic numbers"](https://stackoverflow.com/questions/47882/what-is-a-magic-number-and-why-is-it-bad). So instead of something like `currentAscii <65|| currentAscii>122` it is easier to read (and debug/maintain) when we write code like `currentAscii <'A' || currentAscii>'z'`. (2) Aside from that in `else` branch you are also checking `currentAscii > 122` case which is redundant as such case was already part of "main" `if` condition. – Pshemo Sep 25 '21 at 18:15
  • (3) Your `else` case is basically handling now `A-Z` and `a-z` characters. Now all you need is logic for rotation *within* those ranges separately. You will need to use some modulo operator to calculate rotation (don't want to give you solution since purpose of this task is not to see working code but to *create* it, take your time). – Pshemo Sep 25 '21 at 18:15

1 Answers1

1

Here is an ASCII chart.

Your rotate algorithm is:

If the value is below 65 (less than 'A' in that chart), or above 122 (more than 'z' in that chart), do nothing to the character and just copy it verbatim.

In all other cases, add k, e.g. 11. This goes wrong, immediately. Note how for example [ is in between 65 and 122. Adding 11 to [ is nonsensical. [ should obviously be treated the same as, say, < is treated (which is less than 65).

Of course, adding 11 isn't what 'rotating' is all about: If adding 11 pushes it 'beyond' the z or Z, you want to snap around back to the a or A. Your code does that, but only if the end result is above 'z'. You forgot about 'Z'.

For example, adding 1 to Z (Which is value 90) gets you [ (the character with value 91). Given that 91 is not above 122, you don't modify this at all.

Add more test cases, especially involving capitals, preferably Z. You'll see the problem immediately.

The solution involves getting rid of the 65/122 dichotomy; instead it's 2 ranges: 65-90 and 97-122. For what it is worth, int a = (int) 65; and int a = (int) 'a'; is exactly the same code, and the latter is far more readable, so you might want to use that. There is no need to write any actual number in this code anywhere, except possibly '26'.

rzwitserloot
  • 85,357
  • 5
  • 51
  • 72