0

My code is implementing a Caesar Cipher, and this function is supposed to convert the inputted string, into an encrypted string.

To do this, I have to search for the character in 2 lists, a list of lowercase letters, uppercase letters, and then if it is not a letter, just add the character to the encrypted string.

I decided to use two layers of try's and except's to do this. Is there a better way I could do it, maybe with if/else?

import string from tkinter import * from tkinter import ttk alphaLower = string.ascii_lowercase alphaUpper = string.ascii_uppercase alphaShiftL = alphaLower alphaShiftU = alphaUpper def shiftList(amount): global alphaShiftL global alphaShiftU alphaShiftL = alphaLower[amount:] + alphaShiftL[:amount] alphaShiftU = alphaUpper[amount:] + alphaShiftU[:amount] def encrypt(unencrypted): encrypted = '' for char in unencrypted: #HERE!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!! try: alphaLower.index(char) encrypted += alphaShiftL[alphaLower.index(char)] except ValueError: try: encrypted += alphaShiftU[alphaUpper.index(char)] except ValueError: encrypted += char return encrypted
Improve this question
1
  • Use find instead of index and avoid exceptions altogether. Commented Feb 23, 2016 at 23:17

4 Answers 4

Reset to default
1

There are more efficient ways to implement this -- But working with what you've got, the first thing I might recommend is to concatenate the lists so that you only need 1 try/except:

alpha = alphaLower + alphaUpper alphaShift = alphaShiftL + alphaShiftU for char in unencrypted: try: encrypted += alphaShift[alpha.index(char)] except ValueError: encrypted += char

Now we've only got a single try/except which is nice.

We can do better -- much better -- if we continue working with only a single object that contains the mapping. In this case, what you really want to do is to map a character in alpha to a different character in alphaShift. We can do this trivially with python dict:

mapping = dict(zip(alpha, alphaShift)) for char in unencrypted: encrypted += mapping.get(char, char)

Or, more efficiently:

mapping = dict(zip(alpha, alphaShift)) encrypted = ''.join(mapping.get(char, char) for char in unencrypted)

To "decrypt", you just need the inverse mapping:

inverse_mapping = dict(zip(alphaShift, alpha)) unencrypted = ''.join(inverse_mapping.get(char, char) for char in encrypted)
Improve this answer
Sign up to request clarification or add additional context in comments.

6 Comments

But using one list will introduce some complication in shiftList. Although there is a better way to implement that too, but it will be entirely different from what OP has got.
As an extension to gill's comment; if the letter is z and the shift is 4, your solution would come up with D.
@gill -- I don't think that really complicates it. Maybe it's not really a true Caesar cypher anymore -- It's 2 cyphers stuck together I guess -- But it still works just the same. The important thing is that OP is using multiple lists to map from one character to another. As long as that mapping is well defined and used consistently (and it is), it doesn't matter if it is stored in one list or two. . .
@JackyXie -- That's a generator expression. If you haven't learned about them yet (or their sister constructs list-comprehensions), they're really cool and worth looking up. For the time being, you can consider it to be equivalent to the loop above.
It seems to me that OP wants encryption to preserve case. If so, shiftList needs to be modified (in a more complicate direction) to work with your solution, and that's all I was saying. OP might tell us that he is OK with altering case, but I wouldn't have guessed that from the question.
|
1 
if char.islower(): encrypted += alphaShiftL[alphaLower.index(char)] elif char.isupper(): encrypted += alphaShiftU[alphaUpper.index(char)] else: encrypted += char

See documentation here and here.

Improve this answer

Comments

0

If I understand correctly, you just need to know if something is present in a list. You can test for collection inclusion with this:

if search in collection: ix = collection.index(search) else: //handle no

Did I understand you correctly?

Edit: mgilson's solution because you need the index of the element.

ix = collection.find(search) if ix != -1: //handle yes else: //handle no
Improve this answer

Comments

0

Unless this is an academic exercise, I don't see why you're organizing your code this way. If you feel passionately about keeping the information in arrays, then you can just concatenate them together so you can get by with one lookup.

alphaLower = string.ascii_lowercase alphaUpper = string.ascii_uppercase alpha = alphaLower + alphaUpper alphaShift = alpha def shiftList(amount): global alphaShiftL global alphaShiftU alphaShiftL = alphaLower[amount:] + alphaShiftL[:amount] alphaShiftU = alphaUpper[amount:] + alphaShiftU[:amount] alphaShift = alphaShiftL + alphaShiftU

Now you can do your lookups directly:

def encrypt(unencrypted): encrypted = '' for char in unencrypted: idx = alpha.find(char) encrypted += alphaShift[idx] if idx >= 0 else char return encrypted

However, it would be far better to just use a dictionary:

lc = string.ascii_lowercase uc = string.ascii_uppercase cipher = dict(zip(lc+uc, lc[13:]+lc[:13]+uc[13:]+uc[:13])) def encrypt(input): return ''.join(cipher.get(c,c) for c in input)
Improve this answer

Comments