0

I'm trying to create a function that iterates through a string, finds characters that match to keys in a dictionary and replaces that character with the value in the dictionary for that key. However it currently only replaces first occurrence of a letter that is in the dictionary and stops there, where am I going wrong?

d = {
'I':'1', 'R':'2', 'E':'3', 'A':'4', 'S':'5', 'G':'6', 'T':'7', 'B':'8', 'O':'0',
'l':'1', 'z':'2', 'e':'3', 'a':'4', 's':'5', 'b':'6', 't':'7', 'g':'9', 'o':'0',
}

def cypher(string):
    for i in string:
        if i in d:
            a = string.replace(i,d[i])
            return a

4 Answers4

2

Your return statement is within the if statement, so if a character matches, your function replaces that single character and then returns.

If you want all of your characters replaced, let it iterate through all characters of the string by moving your return statement outside of the for loop.

def cypher(string):
    result = string
    for i in string:
        if i in d:
            result = result.replace(i,d[i])
    return result
zachyee
  • 348
  • 1
  • 8
1

You are prematurely ending your code with the call to return within the for loop. You can fix it by storing your new string outside of the loop, only returning once the loop is done:

def cypher(string):
    a = string  # a new string to store the replaced string
    for i in string:
        if i in d:
            a = a.replace(i, d[i])
    return a

There is something wrong about the logic too, though. If you have a value in your dictionary that is also a key in the dictionary, the key may get replaced twice. For example, if you have d = {'I': 'i', 'i': 'a'}, and the input is Ii, your output would be aa.

Here's a much more concise implementation using join that does not have this problem.

def cypher(string):
    return ''.join(d.get(l, l) for l in string)
Karin
  • 8,404
  • 25
  • 34
1

As zachyee pointed out, your return statement is inside the loop.

You may want to take a look at str.translate, which does exactly what you want:

def cypher(string):
    return string.translate(str.maketrans(d))

Note the use of str.maketrans that transforms your dict in something that string.translate can use. This method is however limited to mappings of single characters.

kjaquier
  • 824
  • 4
  • 11
  • 1
    This actually doesn't work with the given `d` since `translate` requires ordinals as key values: http://stackoverflow.com/questions/17020661/why-doesnt-str-translate-work-in-python-3 – Karin Aug 19 '16 at 22:52
  • 1
    You're right. I edited the code by adding a call to `str.maketrans`. – kjaquier Aug 19 '16 at 22:59
1

One liner -> print ''.join(d[c] if c in d else c for c in s)

Sample Output:

>>> s = 'Hello World' 
>>> d = {
'I':'1', 'R':'2', 'E':'3', 'A':'4', 'S':'5', 'G':'6', 'T':'7', 'B':'8', 'O':'0',
'l':'1', 'z':'2', 'e':'3', 'a':'4', 's':'5', 'b':'6', 't':'7', 'g':'9', 'o':'0',
}
>>> print ''.join(d[c] if c in d else c for c in s)
H3110 W0r1d
ospahiu
  • 3,465
  • 2
  • 13
  • 24