Skip to content

Commit 12ea4bb

Browse files
fix: Caesar cipher produces non-alphabetic output for negative shifts (#7508)
fix: Caesar cipher produced non-alphabetic output for negative shifts normalizeShift cast a possibly-negative int directly to char, which wraps to a huge unsigned 16-bit value (e.g. (char) -1 == 65535). The subsequent char arithmetic then overflowed and emitted characters outside the Latin alphabet, e.g. encode("A", -1) returned '@' instead of 'Z', and the encode/decode round-trip was broken for negative shifts. Normalize the shift into the 0..25 range with ((shift % 26) + 26) % 26 and perform the character arithmetic in int, casting back to char only when appending. Add regression tests covering negative shifts and the encode/decode round-trip. Fixes #7506
1 parent c6fa50f commit 12ea4bb

2 files changed

Lines changed: 37 additions & 17 deletions

File tree

src/main/java/com/thealgorithms/ciphers/Caesar.java

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,8 @@
99
* @author khalil2535
1010
*/
1111
public class Caesar {
12-
private static char normalizeShift(final int shift) {
13-
return (char) (shift % 26);
12+
private static int normalizeShift(final int shift) {
13+
return ((shift % 26) + 26) % 26;
1414
}
1515

1616
/**
@@ -22,21 +22,18 @@ private static char normalizeShift(final int shift) {
2222
public String encode(String message, int shift) {
2323
StringBuilder encoded = new StringBuilder();
2424

25-
final char shiftChar = normalizeShift(shift);
25+
final int shiftChar = normalizeShift(shift);
2626

2727
final int length = message.length();
2828
for (int i = 0; i < length; i++) {
29-
// int current = message.charAt(i); //using char to shift characters because
30-
// ascii
31-
// is in-order latin alphabet
32-
char current = message.charAt(i); // Java law : char + int = char
29+
final char current = message.charAt(i);
3330

3431
if (isCapitalLatinLetter(current)) {
35-
current += shiftChar;
36-
encoded.append((char) (current > 'Z' ? current - 26 : current)); // 26 = number of latin letters
32+
final int shifted = current + shiftChar;
33+
encoded.append((char) (shifted > 'Z' ? shifted - 26 : shifted)); // 26 = number of latin letters
3734
} else if (isSmallLatinLetter(current)) {
38-
current += shiftChar;
39-
encoded.append((char) (current > 'z' ? current - 26 : current)); // 26 = number of latin letters
35+
final int shifted = current + shiftChar;
36+
encoded.append((char) (shifted > 'z' ? shifted - 26 : shifted)); // 26 = number of latin letters
4037
} else {
4138
encoded.append(current);
4239
}
@@ -53,17 +50,17 @@ public String encode(String message, int shift) {
5350
public String decode(String encryptedMessage, int shift) {
5451
StringBuilder decoded = new StringBuilder();
5552

56-
final char shiftChar = normalizeShift(shift);
53+
final int shiftChar = normalizeShift(shift);
5754

5855
final int length = encryptedMessage.length();
5956
for (int i = 0; i < length; i++) {
60-
char current = encryptedMessage.charAt(i);
57+
final char current = encryptedMessage.charAt(i);
6158
if (isCapitalLatinLetter(current)) {
62-
current -= shiftChar;
63-
decoded.append((char) (current < 'A' ? current + 26 : current)); // 26 = number of latin letters
59+
final int shifted = current - shiftChar;
60+
decoded.append((char) (shifted < 'A' ? shifted + 26 : shifted)); // 26 = number of latin letters
6461
} else if (isSmallLatinLetter(current)) {
65-
current -= shiftChar;
66-
decoded.append((char) (current < 'a' ? current + 26 : current)); // 26 = number of latin letters
62+
final int shifted = current - shiftChar;
63+
decoded.append((char) (shifted < 'a' ? shifted + 26 : shifted)); // 26 = number of latin letters
6764
} else {
6865
decoded.append(current);
6966
}

src/test/java/com/thealgorithms/ciphers/CaesarTest.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,29 @@ void caesarDecryptTest() {
3232
assertEquals("Encrypt this text", cipherText);
3333
}
3434

35+
@Test
36+
void caesarEncryptWithNegativeShiftTest() {
37+
// a shift of -1 must wrap 'A' backwards to 'Z', like a shift of +25 would
38+
assertEquals("Z", caesar.encode("A", -1));
39+
assertEquals("z", caesar.encode("a", -1));
40+
assertEquals("EBIIL", caesar.encode("HELLO", -3));
41+
}
42+
43+
@Test
44+
void caesarDecryptWithNegativeShiftTest() {
45+
assertEquals("A", caesar.decode("Z", -1));
46+
assertEquals("HELLO", caesar.decode("EBIIL", -3));
47+
}
48+
49+
@Test
50+
void caesarNegativeShiftRoundTripTest() {
51+
// encode followed by decode with the same shift must return the original text
52+
for (int shift : new int[] {-1, -5, -25, -26, -27, -52}) {
53+
String message = "The quick brown Fox";
54+
assertEquals(message, caesar.decode(caesar.encode(message, shift), shift));
55+
}
56+
}
57+
3558
@Test
3659
void caesarBruteForce() {
3760
// given

0 commit comments

Comments
 (0)