Verschlüsselungsprogramm - Caesar

[xpilz]

Hallo Community .

Ich habe ein Verschlüsselungsprogramm fertig geschrieben und es auch ziemlich oft getestet. Ich hoffe so weit sind keine großen Fehler im Skript enthalten. Auch habe ich das Gefühl, ich habe etwas geschafft. Ich weiß nicht.. Ich betrachte es als mein erstes, richtiges, kleines Projekt.

http://python-forum.de/pastebin.php?mode=view&s=47

Was haltet ihr davon? Hoffe auf konstruktive Kritik.

mfg xpilz

[cofi]

1. Shebang-Zeile sollte besser

Code: Alles auswählen

#!/usr/bin/env python3

lauten
2. Es gibt `string.ascii_*` damit kann man `letters` besser erzeugen, ja auch die gemischte Version
3. `letters` ist eine Modulweite Konstante, also solltest du sie auch so behandeln
4.

Code: Alles auswählen

while value > 52:
    value = value - 52

geht auch als

Code: Alles auswählen

value = value % len(letters)

(die magische Zahl 52 solltest du sowieso ueberall gegen `len(letters)` tauschen)
5. `shift_width` ist ein besserer Name als `value`
6. Du solltest Docstrings nutzen
7. Schau dir mal `argparse` bzw `optparse` an

Da gibts bestimmt noch viel mehr v.a. in der "Verschlüsselung" - der Code zur Entschlüsselung ist btw identisch (sollte er ja auch sein).

Code: Alles auswählen

decode = encode

ist besser als copy+paste. Wenn man Docstrings benutzt ist aber vllt

Code: Alles auswählen

def decode(data, shift_width):
    "Docstring"
    return encode(data,shift_width)

besser.

[xpilz]

Hallo cofi.

Zu 1: Shebang geändert.
2: Ja? Ich habe gesucht aber nichts gefunden. Vorher hatte ich ascii_letters[:26] bzw [26:] benutzt, aber dann hätte ich mehr schreiben müssen. (Jeweils einen Zweig falls der Buchstabe lower bzw. uppercase ist).
3: Aber erst wenn 2 zutrifft, oder?
7. optparse will momentan noch nicht so wie ich. help sieht er bei add_option als builtin Funktion und Parameter ohne zb. -p vorangestellt ist für mich auch noch nicht klar. Werde mir das wohl genauer anschauen müssen.

Danke. Das sowas auch geht wusste ich nicht. Aber eine Zeile ist nicht identisch.

Code: Alles auswählen

[letters.index(dletter) - value*2]

bei encode() wird das addiert.. Ist das falsch?


Und für alles andere Dankeschön.

mfg xpilz

[cofi]

Zu 2

Code: Alles auswählen

>>> import string
>>> string.ascii_lowercase
'abcdefghijklmnopqrstuvwxyz'
>>> string.ascii_uppercase
'ABCDEFGHIJKLMNOPQRSTUVWXYZ'

Zu 3: Nein, dann ist das immer noch eine Konstante, es sei denn du planst zum Ver- und Entschlüsseln unterschiedliche Alphabete zu benutzen.

Zum Ver/Entschlüsseln: Mein Fehler, aber trotzdem kann man den Code auslagern:

Code: Alles auswählen

import string
LETTERS = string.ascii_letters
def _shift(letter, offset = None):
    offset = offset if offset is not None else (len(LETTERS) / 2)
    try:
        return LETTERS[(LETTERS.index(letter) + offset) % len(LETTERS)]
    except ValueError:
        return letter

def encode(data, shift_width):
    shift_width = int(shift_width) % len(LETTERS)
    return "".join(map(lambda x: _shift(x, shift_width), data))

def decode(data, shift_width):
    shift_width = - int(shift_width) % len(LETTERS)
    return "".join(map(lambda x: _shift(x, shift_width), data))

[Dav1d]

Die Ring-Klasse von Leonidas ist auch noch ganz praktisch: http://python-forum.de/viewtopic.php?f=9&t=13080

[xpilz]

@cofi:
Deine Version ist besser. Ich musste zwar noch ein wenig gucken, aber letzendlich hab ich verstanden, wie deine Version funktioniert.

@Dav1d:
Stimmt. Danke.

Ich hätte nicht gedacht das man die Caesar Verschlüsselung mit so wenigen Zeilen bewältigen kann..

mfg, xpilz

[nomnom]

Hallo!
Ich habe heute auch so ein Caesar-Verschlüsselungsprogramm geschrieben:
http://paste.pocoo.org/show/286901/
Das habe ich deswegen geschrieben, weil ich üben wollte, _nicht_, weil ich das Programm brauche.
Ist der Code übersichtlich und verständlich?

Grüße

[DaMutz]

Code: Alles auswählen

alphabet = [] # we'll save the alphabet in here

for char in ascii_lowercase:
    # with this we write the characters of the alphabet into the list
    alphabet.append(char)

ist das selbe wie:

Code: Alles auswählen

alphabet = ascii_lowercase

bei der nächsten Funktion 'def convert_shift(shift):'; kennst du die Funktion 'enumerate'? Die würde die Funktion vereinfachen oder noch einfacher so:

Code: Alles auswählen

ascii_lowercase.find(shift)

ausserdem verwirrt es mich ein bisschen, wenn du 'shift' für 2 verschiedene Dinge verwendest.

Der Rest des Codes, kann jemand anderes kommentieren

[derdon]

Code: Alles auswählen

alphabet = [] # we'll save the alphabet in here

for char in ascii_lowercase:
    # with this we write the characters of the alphabet into the list
    alphabet.append(char)

ist das selbe wie:

Code: Alles auswählen

alphabet = ascii_lowercase

Nein, es ist das selbe wie

Code: Alles auswählen

alphabet = list(ascii_lowercase)

Nichtsdestotrotz ist dieser Codeschnipsel vollkommen überflüssig; die Umwandlung in eine Liste bringt keinen Mehrwert.

[nomnom]

Code: Alles auswählen

alphabet = [] # we'll save the alphabet in here

for char in ascii_lowercase:
    # with this we write the characters of the alphabet into the list
    alphabet.append(char)

ist das selbe wie:

Code: Alles auswählen

alphabet = ascii_lowercase

Nicht ganz, dann ist ``alphabet`` ein String, und dann kann ich auch gleich string.ascii_lowercase benutzen (EDIT: derdon war schneller )

bei der nächsten Funktion 'def convert_shift(shift):'; kennst du die Funktion 'enumerate'? Die würde die Funktion vereinfachen oder noch einfacher so:

Code: Alles auswählen

ascii_lowercase.find(shift)

ausserdem verwirrt es mich ein bisschen, wenn du 'shift' für 2 verschiedene Dinge verwendest.

``enumerate()`` kenne ich nicht, und wo ich nachgeschaut habe, hat sich der Sinn auch nicht erschlossen

[BlackJack]

@nomnom: Der Sinn von `enumerate()` ist, dass man nicht "von Hand" einen Zähler initialisieren und hochzählen muss:

Code: Alles auswählen

In [329]: it = enumerate('hallo')

In [330]: it.next()
Out[330]: (0, 'h')

In [331]: it.next()
Out[331]: (1, 'a')

In [332]: it.next()
Out[332]: (2, 'l')

In [333]: it.next()
Out[333]: (3, 'l')

In [334]: for i, c in enumerate('hallo'):
   .....:     print i, c
   .....:
0 h
1 a
2 l
3 l
4 o

[nomnom]

@nomnom: Der Sinn von `enumerate()` ist, dass man nicht "von Hand" einen Zähler initialisieren und hochzählen muss:

Code: Alles auswählen

In [329]: it = enumerate('hallo')

In [330]: it.next()
Out[330]: (0, 'h')

In [331]: it.next()
Out[331]: (1, 'a')

In [332]: it.next()
Out[332]: (2, 'l')

In [333]: it.next()
Out[333]: (3, 'l')

In [334]: for i, c in enumerate('hallo'):
   .....:     print i, c
   .....:
0 h
1 a
2 l
3 l
4 o

Ah, vielen Dank!

[nomnom]

bei der nächsten Funktion 'def convert_shift(shift):'; kennst du die Funktion 'enumerate'? Die würde die Funktion vereinfachen oder noch einfacher so:

Code: Alles auswählen

ascii_lowercase.find(shift)

ausserdem verwirrt es mich ein bisschen, wenn du 'shift' für 2 verschiedene Dinge verwendest.

muss lauten

Code: Alles auswählen

string.ascii_lowercase.find(shift + 1)

[DaMutz]

ihr habt ja recht, aber es ging mir vor allem um die Idee .

So jetzt weiter:

Code: Alles auswählen

if charnumber > 25:
    charnumber %= 26

diese if Abfrage ist nicht nötig. Ich denke auch nicht, dass dies ein Geschwindigkeitsvorteil bringt...

Code: Alles auswählen

if char in str(range(0,9)) or char in [' ', '\n', '.', ',', ';']:

was ist mit dem Bindestrich '-'? Mach es doch anders herum, prüfe ob es ein Element aus 'alphabet' ist?

[nomnom]

ihr habt ja recht, aber es ging mir vor allem um die Idee .

So jetzt weiter:

Code: Alles auswählen

if charnumber > 25:
    charnumber %= 26

diese if Abfrage ist nicht nötig. Ich denke auch nicht, dass dies ein Geschwindigkeitsvorteil bringt...

War mir nicht aufgefallen, danke!

Code: Alles auswählen

if char in str(range(0,9)) or char in [' ', '\n', '.', ',', ';']:

was ist mit dem Bindestrich '-'? Mach es doch anders herum, prüfe ob es ein Element aus 'alphabet' ist?

Oh Mann, was bin ich denn für einer … daran habe ich gar nicht gedacht! Danke!

[EyDu]

Du hast auch nicht an die neun gedacht

Außerdem solltest du dir die folgenden Unterschiede klar machen:

Code: Alles auswählen

>>> str(range(10))
'[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]'
>>> map(str, range(10))
['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']
>>> string.digits
'0123456789'

[nomnom]

Du hast auch nicht an die neun gedacht

Außerdem solltest du dir die folgenden Unterschiede klar machen:

Code: Alles auswählen

>>> str(range(10))
'[0, 1, 2, 3, 4, 5, 6, 7, 8, 9]'
>>> map(str, range(10))
['0', '1', '2', '3', '4', '5', '6', '7', '8', '9']
>>> string.digits
'0123456789'

Hm, ich hab ``str(range(9))`` einfach ausprobiert, und es hat geklappt, also hab ich mir da keine weiteren Gedanken gemacht. Vielen vielen Dank Es ist dann wohl logischer string.digits oder die Methode mit ``map()`` zu verwenden.

EDIT #1: Doofe Formulierung entfernt

[nomnom]

Da mein Caesar-Programm noch echt lückenhaft war, mal ein Update
http://paste.pocoo.org/show/288933/
Jetzt funktioniert sogar die Entschlüsselung Die hat vorher nicht funktioniert, unbemerkt.

Edit: Ab jetzt muss eine Zahl eingegeben werden …

[EyDu]

Siehst schon deutlich besser aus, aber warum machst du eine Unterscheidung zwischen Groß- und Kleinbuchstaben. Das kann man alles in einem Rutsch erledigen, auch wenn das Ergebnis dann eventuell ein wenig anders ist. Möchtest du deine Codierung beibehalten, dann solltest du doppelten Code in Funktionen auslagen. Und die ganzen "j" wirst du durch die enumerate-Funktion los. Und natürlich möchtest du nicht 26 als magische Zahl benutzen, sonder die Länge der Strings.

Außerdem hätte ich noch etwas zum Nachdenken:

Code: Alles auswählen

>>> alpha = string.ascii_lowercase
>>> offset = 3
>>> dict(zip(alpha, alpha[offset:]+alpha[:offset]))
{'a': 'd', 'c': 'f', 'b': 'e', 'e': 'h', 'd': 'g', 'g': 'j', 'f': 'i', 'i': 'l', 'h': 'k', 'k': 'n', 'j': 'm', 'm': 'p', 'l': 'o', 'o': 'r', 'n': 'q', 'q': 't', 'p': 's', 's': 'v', 'r': 'u', 'u': 'x', 't': 'w', 'w': 'z', 'v': 'y', 'y': 'b', 'x': 'a', 'z': 'c'}
>>> 

Im string-Modul gibt es übrigens auch eine translate-Funktion.

Sebastian

[derdon]

Am einfachsten bleibt aber immer noch str.encode

Code: Alles auswählen

>>> import string
>>> string.printable
'0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~ \t\n\r\x0b\x0c'
>>> string.printable.encode('rot13')
'0123456789nopqrstuvwxyzabcdefghijklmNOPQRSTUVWXYZABCDEFGHIJKLM!"#$%&\'()*+,-./:;<=>?@[\\]^_`{|}~ \t\n\r\x0b\x0c'