Getting an error in a program to encrypt text using Caesar Cipher

Question

Caesar Cipher is a simple way of encryption that shifts a character a number of places in front of it. For example, ABC with a Rotation of 1 would be BCD. In this program, however, the list of all characters include special characters in a random order, i.e.

1234567890-=qwertyuiopasdfghjklzxcvbnm,./~!@#$%^&*()_+|\\{}[]:;\'"QWERTYUIOPASDFGHJKLZXCVBNM

Thus, it is more of a custom cipher but following the principle of Caesar Cipher, the code can be tested by using the 2 functions I have made (encode() and decode()) on the same text, and if the output given is the same as the input, it means that it works. I get an output for some rotation numbers, but some numbers, like 70, give me an error. The code I have written is:

characters = '`1234567890-=qwertyuiopasdfghjklzxcvbnm,./~!@#$%^&*()_+|\\{}[]:;\'"QWERTYUIOPASDFGHJKLZXCVBNM' # All characters in a string, no specific order
key_raw = 70 # One of the keys that gives me an error.
def encode(text, key): # Function that takes in two inputs: Text and key, which is the number of characters to shift forward
    output, text = '', str(text)
    limit = len(characters)
    while key > limit:
        key -= limit # This is my attempt to simplify the key
    for i in text:
        if i in characters:
            output += characters[characters.index(i)+key] # If i is in characters, the rotated character is concatenated to the output
        else:
            output += i # Otherwise, it directly adds the text
    return output
def decode(text, key): # Same thing, except key is subtracted from the index of i in key, as to decrypt it
    output, text = '', str(text)
    limit = len(characters)
    while key > limit:
        key -= limit
    for i in text:
        if i in characters:
            output += characters[characters.index(i)-key]
        else:
            output += i
    return output
print(encode('Why do I get String_Index_Out_Of_Range?', key_raw))

Please let me know where I made an error.

Adeom · Accepted Answer · 2019-12-30T09:31:31.983

Consider what happens when you receive characters.index(i)+key where index is the last symbol in characters.

Simply put, your Caesar cipher is missing modulo operations.

But, to make this more useful, we can still try to improve your code further.

Lets start with the way you build strings. What you are currently using is slower when it comes to large strings. Instead, you should try using .join().

def encrypt(text, key): # Function that takes in two inputs: Text and key, which is the number of characters to shift forward
    output, text = [], str(text)
    limit = len(characters)
    while key > limit:
        key -= limit # This is my attempt to simplify the key
    for i in text:
        if i in characters:
            output.append(characters[characters.index(i)+key]) # If i is in characters, the rotated character is concatenated to the output
        else:
            output.append(i) # Otherwise, it directly adds the text
    return output.join()

Now your key optimization is basically just a modulo operation, so lets replace it with `key = key % len(characters).

def encrypt(text:str, key:int): # Function that takes in two inputs: Text and key, which is the number of characters to shift forward
    output, text = [], str(text)
    key = key % len(characters)
    for i in text:
        if i in characters:
            output.append(characters[characters.index(i)+key]) # If i is in characters, the rotated character is concatenated to the output
        else:
            output.append(i) # Otherwise, it directly adds the text
    return output.join()

Now comes your lookup method. It hopes for O(n) complexity, and has O(nm) complexity. Considering you might encrypt large texts, you may want to trade some space to save time. Lets use mapping structure instead.

And then add some finishing touches to make this prettier.

def encrypt(text:str, key:int, characters:str=characters): 
    '''
    Function that shifts input by the number of buckets specified in key.
    Unknown characters are retained.

    :param text: text to be encrypted
    :param key: number to shift by
    :param characters: string specifying character ordering, None for default.

    :return: encrypted string
    '''
    if not isinstance(text, str) or not isinstance(key, int):
        raise ValueError('Incorrect param type')

    output= []
    l = len(characters)
    key = key % l
    char_map = { characters[i]:((i+key) % l) for i in characters}
    for i in text:
        ch = char_map.get(i)
        if ch is None:
            ch = i
        output.append(ch)
    return output.join()

score 1 · Answer 2 · answered Dec 30 '19 at 09:26

output += characters[characters.index(i)+key]

output += characters[characters.index(i)-key]

The above lines are missing modulo operations.

characters.index(i)+key must be taken under modulo division with length of characters, which is limit in your code. So, it should be,

output += characters[(characters.index(i)+key) % limit]

output += characters[(characters.index(i)-key) % limit]

This should work for you.

Getting an error in a program to encrypt text using Caesar Cipher

2 Answers2