2017-09-07 13 views
1

初めてのPythonプロジェクトのご意見をお寄せいただきありがとうございます。 :DこのPythonコードをより効率的にするにはどうすればいいですか?

は基本的に私はシーザー暗号をコード化していると私は思うのあなたは私が何を意味するか知っていれば、私はコピーして復号化()メソッドのための暗号化()メソッドを貼り付けたので、これはかなりひどく「最適化/効率的」と私が変える唯一のものは、数字をもっと回転させるのではなく、回転させないことでした。これは私が話しているものです:

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

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

私は一種のちょうどnewPositionラインで暗号化()メソッドを呼び出すことができる方法はありますか?私がやったことは正しかったし、修正する必要はない(私は非常に疑う)。

**私が今日始まって以来、私は多くの知識をPythonには持っていない私の頭脳をいくつかの超複雑なコードで吹かないでください。ありがとうございました!!! **

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

コードレビューを探しているようです。それらは[別のサイト](https://codereview.stackexchange.com/)に移動します。 – user2357112

+0

ありがとう、このウェブサイトを提供してくれてありがとう!! – Kieran

答えて

3

一般に、str.findはパフォーマンスが悪いです。それはO(n)の複雑さですが、ひどいことではありませんが、実際にはほとんど必要ありません。この場合、ordを使用して各文字を序数に変換し、ord('a')を引いて97-122の代わりに0-25を得ることができます。

これは、chrを使用してルックアップを必要とせずに元に戻すことができるため、特に便利です。

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 

も注意+=で文字列を連結する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) 

そして、それchr(((ord(letter) - ord('a') + key) % 26) + ord('a'))以来

はとても醜いですが、私は関数にそれをリファクタリングしたいです。

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) 

保守のポイント:あなたの結果から、あなたの入力を区切るかどうかはtestably良いコードを書く方が簡単です。今のところあなたの関数の単体テストを書くには、stdinのいくつかの猿パッチをしなければならないでしょうが、ユーザー入力の要求をmainに移動してそれぞれの関数から外すとはるかに簡単になります。

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

をし、まったくdecrypt関数を記述していない:あなたはシーザー暗号は、鍵の符号を反転させることにより、可逆的である考えるとき

実際に
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. 

、あなたは簡単に行うことができます!

+1

OMG !!あまりにも多くの人ありがとう!私は真剣にあなたが私を助けるあなたの時間を取っていただきありがとうございます! – Kieran

+1

@TemporalWolfありがとう! –

関連する問題