How can I solve a problem with caesar code in Python

Question

I am trying to create a simple Caesar Code function that has to decipher a string given in input.

Clear text = A B C D E F G H I J K L M N O P Q R S T U V W X Y Z

Encrypted= D E F G H I J K L M N O P Q R S T U V W X Y Z A B C

This is my code to DECIPHER:

def deciph(s):
    b='abcdefghijklmnopqrstuvwxyz'
    a='defghijklmnopqrstuvwxyzabc'
    for i in s:
        for j in range(len(a)):
            if i==a[j]:
                s=s.replace(i,b[j])
    return s

This code works almost always fine, for example:

deciph('vxq') --> 'sun'

deciph('ohwwhu') --> 'letter'

The problems are those of this case:

deciph('sp')--> 'mm'  #should be 'pm'

deciph('ol')-->'ii'  #should be 'li'

and so when, tha first letter, deciphered, is equal to the second letter encrypted.

How can I modify my code? where is the mistake? I know that on internet there are many other ways to do the same exercise, but now I am interested to understand the mistake of MY code.

Answer 1

Let's use your example of deciph('sp'). The problem is when you do s=s.replace(i,b[j]), you are initally replacing all instances of 's' with 'p', giving you s = 'pp', and then on the second pass, replacing all instances of 'p' with 'm', giving you s = 'mm'. This could be rectified by holding a new initial variable rv = '', and then using rv+=b[j] for each letter instead of altering s. Then at the end, just return rv.

Answer 2

As already mentionned, the issue you observe is due to inconditionnaly replacing each char in s by the "clear" one - since a and b are composed of the same letters and str.replace(x, y) replaces ALL occurences of x with y, some of the chars are "deciphered" again and again...

The first step toward a proper solution is to build a new string manually instead of using str.replace:

def decipher(s):    
    result = []
    for char in s:
        for index, crypted in enumerate(a):
            if char == crypted:
                result.append(b[index])
                # no need to go further
                break
        else:
            # The else clause of a for loop is only executed
            # if the for loop runs to the end without being
            # interrupted by a break statement.
            #
            # Here we use it to handle the case of whitespaces 
            # or any other char that's in `s` but not in `a` 

            result.append(c)

    return "".join(result)

Now while this will yield the expected results, it's uselessly complicated and very inefficient. What you're doing is, essentially, mapping - in this case, mapping encrypted characters to decrypted one -, so the obvious solution here is to use Python's main mapping builtin type: a dict:

CLEAR = 'abcdefghijklmnopqrstuvwxyz'
CRYPT = 'defghijklmnopqrstuvwxyzabc'

DECIPHER_MAP = dict(zip(CRYPT, CLEAR))

def decipher(s):
    ## the 'unrolled' version:
    # result = []
    # for c in s:
    #     result.append(DECIPHER_MAP.get(c, c))
    # return "".join(result)

    # which is even simpler _and_ faster (and uses less memory)
    # using a generator expression.

    return "".join(DECIPHER_MAP.get(c, c) for c in s)

As you can see, the algorithm is much simpler. It's also much faster (a dict lookup is 0(1) and highly optimized), and eats less memory (no ned for the intermediate result list).

In programming, choosing the proper data structure is key...

NB:

now I am interested to understand the mistake of MY code.

This is very laudable, to say the least - I whish everyone here would do the same. Now that's something you could have sorted out by yourself with the most simple and basic debugging technic: tracing code execution by printing out your current state at strategic moments:

def deciph(s):    
    for i in s:
        print("i : '{}' - s : '{}'".format(i, s))
        for j in range(len(a)):
            if i==a[j]:
                print("i == '{}' - replacing with '{}'".format(i, b[j]))
                s = s.replace(i, b[j])
                print("now s : '{}'".format(s))   
    return s


>>> deciph("pme")
i : 'p' - s : 'pme'
i == 'p' - replacing with 'm'
now s : 'mme'
i : 'm' - s : 'mme'
i == 'm' - replacing with 'j'
now s : 'jje'
i : 'e' - s : 'jje'
i == 'e' - replacing with 'b'
now s : 'jjb'
'jjb'

Answer 3

You can use find() method to find the position of characters in a loop.

def deciph(s):
    b='abcdefghijklmnopqrstuvwxyz'
    a='defghijklmnopqrstuvwxyzabc'
    decrypted = ''
    for i in s:
       pos = a.find(i)
       if pos != -1:
         decrypted += b[pos]
       else:
          pass # Here you can throw an error
          # Or you can directly pass it to the decrypted string
          # since you want spaces and other punctuation to remain
          # decrypted += i
    return decrypted