@fedjan: Du solltest trotzdem die Werte nicht selbst in die Zeichenkette hinein formatieren. Selbst wenn man SQL-Injection nahezu ausschliessen kann, können immer noch normale Daten dabei sein, die zu einem Syntaxfehler in der SQL-Anweisung führen oder andere Probleme verursachen. Da scheinen ja zum Beispiel (Firmen)Namen in den Daten zu sein. "Peterchen's Dorfapotheke" würde beispielsweise ein Problem darstellen.
Das `string`-Modul sollte man nun seit sehr vielen Jahren schon nicht mehr für Funktionen verwenden, die es auch als Methoden auf Zeichenketten gibt. Ausserdem ist `find()` hier die falsche Wahl. Die Methode sollte man nur verwenden, wenn man tatsächlich den Index wissen möchte. Selbst dafür würde ich `index()` bevorzugen. Falls man nur wissen möchte ob eine Zeichenkette in einer anderen enthalten ist oder nicht, dann ist der ``in``- beziehungsweise ``not in``-Operator einfacher geschrieben und für den Leser auch offensichtlicher. Aber wenn ich Dein Datenformat noch richtig im Kopf habe, möchtest Du gar nicht wissen ob die Zeichenketten *irgendwo* vorkommen, sondern die stehen *immer* am Anfang der Zeile. Um das zu testen haben Zeichenketten eine eigene Methode: `startswith()`.
Die ``while``-Schleife mit `kc` in der Bedingung ist ziemlich eindeutig eine ``for``-Schleife.
Die `readline()`-Methode würde ich grundsätzlich meiden. Dateiobjekte sind iterierbar und haben eine `next()`-Methode die grundsätzlich das gleiche Ergebnis liefert. Allerdings sind die beiden Wege nicht zwingend kompatibel, dass heisst man sollte `readline()` und `next()` nicht mischen, da können Daten bei ”verschluckt” werden. Zumindest in CPython sollte `next()` effizienter sein und man kann das Dateiobjekt als ”iterable” verwenden. Zum Beispiel um in der `kc`-Schleife den unnötigen Zähler los zu werden in dem man direkt von der Datei mit `itertools.islice()` die gewünschte Anzahl von Zeilen abfragt.
Ich würde mal raten, dass diese Schleife bisher bei Dir noch nie mehr als einmal durchlaufen wurde. Dann produziert sie nämlich eine syntaktisch falsche 'WHERE'-Bedingung, denn die Einzelbedingungen müssten mit 'AND' verknüpft werden.
Letztlich ist der Code viel zu „direkt” und zu „kurzsichtig” oder „greedy”. Statt erst einmal die Daten aus der Eingabe zu extrahieren und zu sammeln, wird immer sofort versucht mit jedem kleinen Stück eine Ausgabe zu basteln. Das führt hier zu einem grossen, unübersichtlichen „Klumpen” Quelltext mit unnötigen Wiederholungen und einer gewissen Ineffizienz. Zum Beispiel das für einen Datensatz mehrere Spaltenänderungen einzeln ausgeführt und `commit()`\et werden. Und das Verarbeiten und aufteilen einer Zeile wird zumindest sehr ähnlich in dem gezeigten Quelltext zwei mal gemacht. Ich vermute mal dass das so ähnlich noch öfter an anderen Stellen im Quelltext wiederholt wird. So etwas gehört in eine Funktion ausgelagert. Beziehungsweise habe ich in einem anderen Thread ja schon mal die Vermutung geäussert, dass die Zeilen ganz grundsätzlich alle so aufgebaut sein könnten, dass am Anfang eine zweistellige Zahl steht. Dann sollte man das auch als allererstes und an *einer* Stelle im Quelltext verarbeiten.
Es gibt für meinen Geschmack relativ viele unnötige Klammern um Ausdrücke in dem Quelltext.
Ich lande dann bei dem Quelltext ungefähr bei so etwas (was immer noch kein gut organisierter Code ist):
Code: Alles auswählen
from itertools import chain, islice
ATTRIB_SPALTE = 1
# ...
def parse_line(line):
return int(line[:2]), line[2:]
def get_attribute_name(attributes, index):
return attributes[index - 1][ATTRIB_SPALTE]
# ...
while line.startswith('00U'):
where_conditions = list()
for line in islice(infile, keysCounter):
index, value = parse_line(line)
name = get_attribute_name(attribute_koplett, index)
where_conditions.append((name, value))
new_values = list()
while not line.startswith(('00U', '00E')):
line = infile.next()
index, value = parse_line(line)
if index:
name = get_attribute_name(attribute_koplett, index)
new_values.append((name, value))
sql = 'UPDATE {0} SET {1} WHERE {3}'.format(
m.table_name,
', '.join(
'{0} = ?'.format(replace_all(n, ucode))
for n, _ in new_values
),
' AND '.join(
'{0} = ?'.format(replace_all(n, ucode))
for n, _ in where_conditions
)
)
all_values = chain.from_iterable(
(replace_all(v, ucode) for _, v in xs)
for xs in [new_values, where_conditions]
)
cursor.execute(sql, all_values)
connection.commit()
Die Einrücktiefe der ersten ``while``-Schleife lässt allerdings Böses ahnen — nämlich dass Du den ganzen komplexen Zustandsautomaten aus dem
Skript zum Einlegen von Tabellen-Thema jetzt in eine riesige Funktion gesteckt hast die *alles* macht. Also Eingabe, Verarbeitung, und Ausgabe in einem. Das ist so gar nicht das was man eigentlich möchte um den Überblick nicht zu verlieren. Das Programm könnte sicher auch von objektorientierter Programmierung profitieren, anstelle von verschachtelten Datenstrukturen mit magischen Indizes auf Werte in Listen.
