Das deutsche Python-Forum

Versuche gerade, typische "konventionelle" Verschlüsselungsverfahren (z.B. Matrix-Transformation, Vigenère) zu implementieren.

Dabei versuche ich meinen Code möglichst funktional zu halten. Sobald es allerdings zwei dimensional wird, komme ich mit FP nicht mehr weiter.

Außerdem hab ich noch ein paar Design-Schwierigkeiten, z.B. bei der vernünftigen Benennung von Funktionen.

Vielleicht habt ihr ein paar Tipps, hier die entsprechenden Problemstellen:

Description Resource Path Location Type
ID:W0511 TODO: beserer Name crypt.py /Crypt line 63 PyLint Problem
ID:W0511 TODO: beserer Name! crypt.py /Crypt line 78 PyLint Problem
ID:W0511 TODO: functional style possible? crypt.py /Crypt line 129 PyLint Problem
ID:W0511 TODO: bessere Beschreibung? crypt.py /Crypt line 155 PyLint Problem
ID:W0511 TODO: FP style?? crypt.py /Crypt line 195 PyLint Problem
ID:W0511 TODO: FP style and error correction crypt.py /Crypt line 238 PyLint Problem
ID:W0511 TODO: FP ? crypt.py /Crypt line 254 PyLint Problem

Hier der Code: http://pastebin.com/f5bc0706a
Und Aufruf-Beispiele http://pastebin.com/f71578cb4 mit entsprechender Ausgabe http://pastebin.com/f75e943c7

Hallo.

Ich sehe Funktionen, aber das ist noch lange nicht funktional. Namen und Dokumentation haben auch nichts (direkt) mit dem Design zu tun. Eine vernünftige Namensvergabe und Beschreibung lernt man ganz gut durch Vergabe von Namen oder beschreiben

Verwende einfach folgende Richtlinie: Wenn dir kein Name/keine Beschreibung einfällt oder nur ein sehr langer, dann denke noch einmal über die Funktion nach. Mit großer Wahrscheinlichkeit ist sie dann nicht sinnvoll oder sie tut zu viel.

Du solltest dir auch angewöhnen die Logik von der Ausgabe zu trennen. Zum Nachvollziehen ist es ja mal ganz hilfreich, aber in einem richtigen Modul hat das in der Form nichts zu suchen.

Und noch ein wenig zu deinen Frage:

Code: Alles auswählen

matrix = [ ['-'] * cols for i in xrange(height)]

Statt einem "-" willst du aber sicher None verwenden. Am besten benutzt du aber gleich eine richtige Matrix, du verwendest doch eh schon numpy.

Code: Alles auswählen

text = [c for c in text[len(solution):]]

Einfach so (ich gehe mal davon aus, dass "text" ein String ist):

Code: Alles auswählen

text = text[len(solution):]

Mal noch ein paar weitere Anmerkungen:
- Wenn du funktional arbeiten willst: warum sind nicht einmal "itertools" und "functools" importiert?
- Du hast überall beschrieben, was deine Funktionen machen. Aber nicht, was sie als Parameter erwarten.
- Zu Zeile 14: Es gibt das [mod]string[/mod]-Modul für solche Konstanten.
- Zeilen 66 bis 69:

Code: Alles auswählen

dict(zip(ALPHABET, ALPHABET[shift:]+ALPHABET[:shift]))

- Zeile 70: ''.join(map(replace_map.get, code))
- Zeile 105-107: Strings haben eine "index"-Methode. Ich würde die Methode nicht auf das "e" spezialisieren. Ich weiss, dass du nur das e brauchst, aber jeden möglichen Buchstaben zu testen ist nicht schwerer.
- Zeile 115: Die 1 als Startwert müsstest du eigentlich weglassen können.
- Zeile 142 bis 147:

Code: Alles auswählen

"".join("".join(reversed(line) if index%2 else line) for index, line in enumerate(matrix))

Sind jetzt vielleicht aber schon mehr als 80 Zeichen.
- 158 : Die Zeile ist nicht dein Ernst, oder? "list()" existiert. Außerdem ist "keyword_list" ein schlechter name. Warum nicht "keywords"?
Meinst du mit der ganzen Funktion vielleicht das hier:

Code: Alles auswählen

[ALPHABET.index(char)+1 for char in "THISISATEXT"]

Etwas stutzig macht mich, dass der Index bei 1 anfängt. Du solltest besser bei der 0 bleiben. Vielleicht überblicke ich aber auch gerade nicht alle Abhängigkeiten.
- 174: Die Unterscheidung würde ich mir sparen und Tupel erzeugen. Diese kannst du dann bei Bedarf noch in der Reihenfolge drehen und daraus ein Dictionary erzeugen.
- 193: Die Klammern bei "(key_char, text_char)" kannst du weglassen. Kommas erzeugen Tupel, nicht Klammern.
- 194: Der Aufruf von "str" ist überflüssig. Strings sind unveränderbar.
- 199: Warum keine for-Schleife?
- 202-205: Die Unterscheidung kannst du schon vor der Schleife machen.
- 235: Schon wieder so ein kranker Aufruf ^^
- 226: Die Funktion sieht wirklich grausam aus. Viel zu lang, keine Kommentare, hunderte von magischen Zahlen und ganz viel doppelten Code. Die lese ich nicht weiter

Ich hoffe, dass die Tipps ein wenig helfen.

@ice2k3: Ein besserer Name für `replace_shifted_alphabet()` wäre vielleicht `rotate()`. Und Du nennst IMHO zuviel `code`. Denn da wird ja kein Code übergeben, sondern "plaintext" oder "ciphertext". Bei Funktionen die sowohl ver- als auch entschlüsseln, könnte man es dann bei `text` belassen. `shift_position` würde ich bei `rotate()` dann `distance` nennen.

Es würde den Quelltext vereinfachen, wenn Du das `ALPHABET` zweimal hintereinander in der Zeichenkette hättest. Dann kann man auf ``% len(ALPHABET)`` verzichten.

Für `try_e_shift_alphabet()` fällt mir auf Anhieb auch kein besserer Name ein, aber ich würde das 'e' nicht hart kodieren. Insbesondere kommt das 'e' in der Funktion ja gar nicht vor. Das sollte man dann auch an `find_shift_position()` weitergeben und dort halt auch nicht davon ausgehen, dass es schon an Index 4 zu finden ist. Ich hätte in diese Funktion keine Ausgabe eingebaut, und es auch nicht auf zwei Versuche begrenzt, sondern einen Generator geschrieben, der alle durchprobiert. Wenn sich eine Funktion mit Ausgabe dann nur die ersten zwei davon nimmt, ist das ja auch okay. Also ungefähr so (alles ungetestet):

Code: Alles auswählen

def try_decoding(text, expected_highest='E'):
    histogram = sorted(count_chars(text).items(),
                       key=itemgetter(1),
                       reverse=True)
    for char, dummy in histogram:
        yield rotate(text, get_distance(expected_highest, char))

def get_distance(char_1, char_2):
    return ALPHABET.index(char_1) - ALPHABET.index(char_2)

Die `get_distance()` ist dabei Deine `find_shift_position()` deren Schleife man auf jeden Fall mit der `index()`-Methode ersetzen kann.

In der `try_replace_shifted_alphabet()` ist wieder Ausgabe enthalten. Ohne Ausgabe und typisch funktional wäre wohl das hier:

Code: Alles auswählen

def try_all_possibilities(text):
    return imap(partial(rotate, text), xrange(1, len(ALPHABET)))

Mit `functools.partial()` und `itertools.imap()`.

Für `decode_matrix_trans()` gibt es sicher einen funktionalen Weg, denn letztendlich ist das ja nur ein anderes Paradigma, aber genauso mächtig wie prozedurale Programmierung. Du musst halt die Abbildung von einem Zeichen auf das andere in einer "geschlossenen Formel" ausdrücken. Ob das bei `decode_matrix_trans()` halbwegs lesbar und nachvollziehbar hinzubekommen ist, möchte ich jetzt, so spät, nicht mehr ausprobieren.

`generate_rank()` braucht definitiv eine bessere Beschreibung. Mich hat sie jedenfalls eher verwirrt als erleuchtet.

Beim Namen `keyword_list` hätte ich eine Liste von Schlüsselwörtern erwartet und nicht einzelne Buchstaben. Diese Liste ist ausserdem überflüssig, man kann dafür an jeder Stelle in der Funktion auch `keyword` direkt verwenden. `position` würde ich `positions` nennen, oder `char2last_index`, das beschreibt besser was da tatsächlich drin steckt. Ausserdem geht das kürzer:

Code: Alles auswählen

def generate_rank(keyword):
    char2last_index = dict((c, i) for i, c in enumerate(sorted(keyword)))
    return [char2last_index[c] + 1 for c in keyword]

So weiter mag ich jetzt nicht mehr lesen -- muss dringend ins Bett.

BlackJack hat geschrieben:Insbesondere kommt das 'e' in der Funktion ja gar nicht vor.

Er hat es nur gut versteckt

Das deutsche Python-Forum

Functional Programming (FP) bei Verschlüsselungsalgorithmen

Functional Programming (FP) bei Verschlüsselungsalgorithmen