12
\$\begingroup\$

I have been trying to implement various substitution ciphers in python. I implemented the Baconian cipher using a dictionary in which the plaintext letters are keys and the ciphertext letters are the corresponding values. Is there a way to make the following code shorter or more efficient or is there a better approach to implementing the cipher.

Python program to implement Baconian cipher

'''This script uses a dictionary instead of 'chr()' & 'ord()' function'''

'''
Dictionary to map plaintext with ciphertext
(key:value) => (plaintext:ciphertext)
This script uses the 26 letter baconian cipher
in which I,J & U,V have distinct patterns
'''
lookup = {'A':'aaaaa', 'B':'aaaab', 'C':'aaaba', 'D':'aaabb','E':'aabaa',
    'F':'aabab', 'G':'aabba', 'H':'aabbb', 'I':'abaaa', 'J':'abaab',
    'K':'abaab', 'L':'ababa', 'M':'ababb', 'N':'abbaa', 'O':'abbab',
    'P':'abbba', 'Q':'abbbb', 'R':'baaaa', 'S':'baaab', 'T':'baaba',
    'U':'babaa', 'V':'babab', 'W':'babaa', 'X':'babab', 'Y':'babba' 
    'Z':'babbb'}
#Function to encrypt the string according to the cipher provided
def encrypt(message):
    cipher = ''
    for letter in message:
        #checks for space
        if(letter != ' '):
            #adds the ciphertext corresponding to the plaintext from the dictionary
            cipher += lookup[letter]
        else:
            #adds space
            cipher += ' '

    return cipher

#Function to decrypt the string according to the cipher provided
def decrypt(message):
    decipher = ''
    i = 0

    #emulating a do-while loop
    while True :
        #condition to run decryption till the last set of ciphertext
        if(i < len(message)-4):
            #extracting a set of ciphertext from the message
            substr = message[i:i+5]
            #checking for space as the first character of the substring
            if(substr[0] != ' '):
                '''
                This statement gets us the key(plaintext) using the values(ciphertext)
                Just the reverse of what we were doing in encrypt function
                '''
                decipher += list(lookup.keys())[list(lookup.values()).index(substr)]
                i += 5 #to get the next set of ciphertext
            else:
                #adds space
                decipher += ' '
                i += 1 #index next to the space
        else:
            break #emulating a do-while loop

    return decipher

def main():
    message = "ALICE KILLED BOB"
    result = encrypt(message.upper())
    print (result)

    message = "aaaaaababaabaaaaaabaaabaa abaababaaaababaababaaabaaaaabb aaaababbabaaaab"
    result = decrypt(message.lower())
    print (result)

#Executes the main function
if __name__ == '__main__':
    main()

I have been implementing most of the substitution ciphers using dictionaries and I want to know is there a another approach to this as I am bored of using dictionaries.

\$\endgroup\$
14
\$\begingroup\$

Bug

Letters J & K, U & W, and V & X have the same pattern. This is not what the specification says: either I & J and U & V shares the same pattern, or none of the letters do.

Style

String literals ain't comments. If you want two lines of comments, you start your two lines with a # each. Also take the habit to put a space after the # sign, it eases readability.

You also don't need to put parentheses around simple conditions.

Lastly, PEP 8 recommends to use ALL_CAPS to name constants, like LOOKUP.

Generators

Concatenating strings using += is slow as strings are immutables and Python need to copy the whole string each time you add a bit at the end. For better performances, it is advised to use ''.join. To do this, it is easier to separate each function into two: one that iterates over the input and joins the converted output, the other that generates the converted output. Something like:

def encrypt_letter(letter):
    """Convert a plain-text ascii letter into its Bacon's form.

    Let symbols and non ascii letter fail through gracefully.
    """
    return LOOKUP.get(letter.upper(), letter.lower())


def encrypt(message):
    """Encrypt a message using Bacon's cipher"""
    return ''.join(map(encrypt_letter, message))

One more thing to note with this implementation: you shouldn't require that the input is properly formatted, you should enforce it in your functions (usage of upper and lower within your function, not before its call)

Do-while

In Python, a do-while loop is emulated using something along the lines of:

while True:
    do_something()
    if ...:
        break

In your case, having the if directly as the first action of the while and an else: break, you just need a simple while loop:

while i < len(message) - 4:
    substr = message[i:i + 5]
    ...
\$\endgroup\$
  • \$\begingroup\$ Thanks, this is what I was looking for. Since I am new to python I'm not aware of the best practices. I will keep these points in mind the next time I write another python script. \$\endgroup\$ – palash25 Jun 26 '17 at 8:37
  • 5
    \$\begingroup\$ About the string literals not being comments: docstrings are the notable exception. But how string literals are used here, is not as docstring. \$\endgroup\$ – Mast Jun 26 '17 at 10:03
10
\$\begingroup\$

In Python, whenever you are adding more than two strings, you are probably doing it wrong. This is because when you add two strings, it needs to create a new string, copy the contents of the first string there and then copy the second string there. This is relatively expensive, so should be avoided. You can just accumulate the strings in a list and then str.join them at the end.

In Python you don't need () around the expression in if, for or while statements.

Python has a way to add documentation to a function (or class), they are called docstrings. How they should look like is set down in PEP257, but this is the most simple form:

def f(a, b):
    """Returns the sum of `a` and `b`."""
    return a + b

In your comments above the functions (which I will turn into docstrings below), you state that the functions en/decrypt the message using the given cipher. But both functions don't actually accept any cipher. So you need to either change the function signature or the docstring.

You were also missing a , at the end of your second to last line in lookup.

For the decrypt function, I would first reverse the cipher and then use that to decrypt the message.

I would also add ' ': ' ' to the dictionary so we don't need to make any special case for space while encrypting.

With these edits, your code becomes:

lookup = {'A': 'aaaaa', 'B': 'aaaab', 'C': 'aaaba', 'D': 'aaabb', 'E': 'aabaa',
          'F': 'aabab', 'G': 'aabba', 'H': 'aabbb', 'I': 'abaaa', 'J': 'abaab',
          'K': 'abaab', 'L': 'ababa', 'M': 'ababb', 'N': 'abbaa', 'O': 'abbab',
          'P': 'abbba', 'Q': 'abbbb', 'R': 'baaaa', 'S': 'baaab', 'T': 'baaba',
          'U': 'babaa', 'V': 'babab', 'W': 'babaa', 'X': 'babab', 'Y': 'babba',
          'Z': 'babbb', ' ': ' '}


def encrypt(message, cipher):
    """Ecrypt the string according to the cipher provided."""
    # return ''.join(map(lookup.get, message))
    encrypted = []
    for letter in message:
        encrypted.append(lookup[letter])
    return ''.join(encrypted)


def decrypt(message, cipher):
    """Decrypt the string according to the cipher provided."""
    plaintext = []
    i = 0
    reverse_cipher = {v: k for k, v in cipher.items()}
    # emulating a do-while loop
    while True:
        # condition to run decryption till the last set of ciphertext
        if i < len(message) - 4:
            # extracting a set of ciphertext from the message
            substr = message[i:i + 5]
            # checking for space as the first character of the substring
            if substr[0] != ' ':
                # This statement gets us the key(plaintext) using the values(ciphertext)
                # Just the reverse of what we were doing in encrypt function
                plaintext.append(reverse_cipher[substr])
                i += 5  # to get the next set of ciphertext
            else:
                # adds space
                plaintext.append(' ')
                i += 1  # index next to the space
        else:
            break  # emulating a do-while loop

    return ''.join(plaintext)


def main():
    message = "ALICE KILLED BOB"
    result = encrypt(message.upper(), lookup)
    print(result)

    message = "aaaaaababaabaaaaaabaaabaa abaababaaaababaababaaabaaaaabb aaaababbabaaaab"
    result = decrypt(message.lower(), lookup)
    print(result)

# Executes the main function
if __name__ == '__main__':
    main()

Your encrypt function can be greatly simplified when using map. map goes through an iterable and applies a function to all values. As a function to apply we can use the dict.get function of lookup.

def encrypt(message, cipher):
    """Ecrypt the string according to the cipher provided."""
    return ''.join(map(lookup.get, message))

You could also make the decrypt function shorter using list comprehensions, but it is not so readable in that case:

def decrypt(message, cipher):
    reverse_cipher = {v: k for k, v in cipher.items()}
    return ' '.join(''.join(reverse_cipher[word[i:i + 5]]
                            for i in range(0, len(word), 5))
                    for word in message.split(' '))
\$\endgroup\$
  • \$\begingroup\$ That point on how to avoid making a special case for space was very helpful. I hadn't thought of that. Thanks. \$\endgroup\$ – palash25 Jun 26 '17 at 8:54

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.