2017-09-07 2 views
1

J'apprécierais beaucoup vos commentaires sur mon tout premier projet Python! : DComment rendre ce code Python plus efficace?

Fondamentalement, je suis en train de coder un César Cipher et je pense que c'est assez "optimisé/efficace" si vous voyez ce que je veux dire, parce que j'ai copié et collé la méthode encrypt() pour la méthode decrypt() la seule chose que je change était de faire tourner les nombres plus, je les ai moins tournés. Voilà ce que je parle:

 newPosition = (abc.find(letter) - key) % 26 

^^ Instead of having a + (plus) I made it a - (minus) ^^ 

Est-il possible que je peux appeler en quelque sorte la méthode Encrypt() à seulement la ligne de nouvPosition? Ou ce que j'ai fait était correct et il n'a pas besoin de réparation (ce dont je doute fortement)

** S'il vous plaît n'oubliez pas que je n'ai pas beaucoup de connaissances en Python (le cas échéant) depuis que je viens de commencer aujourd'hui alors ne me mets pas au cerveau avec un code super complexe. JE VOUS REMERCIE!!! **

abc = 'abcdefghijklmnopqrstuvwxyz' 

def main(): 
    message = input("Would you like to encrypt or decrypt a word?") 
    if message.lower() == "encrypt": 
     encrypt() 
    elif message.lower() == "decrypt": 
     decrypt() 
    else: 
     print("You must enter either 'encrypt' or 'decrypt'.") 
     main() 


def encrypt(): 
    message = input("Enter a message to encrypt: ") 
    message = message.lower() 
    key = int(input("What number would you like for your key value?")) 
    cipherText = "" 
    for letter in message: 
     if letter in abc: 
      newPosition = (abc.find(letter) + key) % 26 
      cipherText += abc[newPosition] 
     else: 
      cipherText += letter 
    print(cipherText) 
    return cipherText 

def decrypt(): 
    message = input("Enter a message to decrypt: ") 
    message = message.lower() 
    key = int(input("What number would you like for your key value?")) 
    cipherText = "" 
    for letter in message: 
     if letter in abc: 
      newPosition = (abc.find(letter) - key) % 26 
      cipherText += abc[newPosition] 
     else: 
      cipherText += letter 
    print(cipherText) 
    return cipherText 

main() 
+5

Il semble que vous cherchiez une révision de code. Ceux-ci vont sur un [site différent] (https://codereview.stackexchange.com/). – user2357112

+0

Oh ok, merci de fournir ce site !! – Kieran

Répondre

3

En général, str.find est mauvais en termes de performances. C'est la complexité de O (n), ce qui n'est pas terrible, mais vous en avez rarement vraiment besoin. Dans ce cas, vous pouvez utiliser ord pour convertir chaque lettre en son ordinal, puis soustraire ord('a') pour obtenir 0-25 au lieu de 97-122. Ceci est particulièrement utile, car vous pouvez ensuite utiliser chr pour effectuer une conversion sans avoir besoin d'une recherche.

for letter in message: 
    if letter in string.ascii_lowercase: # same as "abcdef..z" 
     new_position = ((ord(letter) - ord('a') + key) % 26) + ord('a') 
     new_ch = chr(new_position) 
     ciphertext += new_ch 

Notez également que les chaînes concaténer avec += est pas aussi vite que quelque chose comme str.join.

new_letters = [chr(((ord(letter) - ord('a') + key) % 26) + ord('a')) if letter in ascii_lowercase else letter for letter in message] 
ciphertext = "".join(new_letters) 

Et depuis que chr(((ord(letter) - ord('a') + key) % 26) + ord('a')) est si laid, je Refactor cela en fonction.

def rotate(letter, key=0): 
    c_pos = ord(letter) - ord('a') 
    rotated = c_pos + key 
    modded = rotated % 26 
    final_pos = modded + ord('a') 
    return chr(final_pos) 

new_letters = [rotate(c, key) if c in string.ascii_lowercase else c for c in letters] 
ciphertext = "".join(new_letters) 

Un point de maintenabilité: il est plus facile d'écrire testably bon code si vous séparez vos entrées de vos résultats. À l'heure actuelle, vous devrez effectuer des correctifs de stdin pour écrire des tests unitaires pour n'importe laquelle de vos fonctions, mais si vous déplacez vos demandes d'entrée d'utilisateur dans main et que vous ne les utilisez pas, cela devient beaucoup plus facile.

def main(): 
    message = input("What's the message to encrypt/decrypt? ") 
    key = int(input("What number would you like for your key value? ")) 
    choice = input("Choose: encrypt or decrypt. ") 
    if choice == "encrypt": 
     result = encrypt(message, key) 
    elif choice == "decrypt": 
     result = decrypt(message, key) 
    else: 
     # something here about a bad user input. 

En fait, si l'on considère le Caesar Cipher est réversible en renversant le signe de la clé, vous pouvez simplement faire:

if choice == "encrypt": 
     result = encrypt(message, key) 
    elif choice == "decrypt": 
     result = encrypt(message, key * (-1)) 

et ne pas écrire une fonction decrypt du tout!

+1

OMG !! Merci beaucoup les gars!!! J'apprécie vraiment que vous preniez votre temps pour m'aider !! – Kieran

+1

@TemporalWolf merci! –