global variable in for Schleife

[m6T]

Hallo

Also, ich versuche einige Werte, die über eine for schleife aus einer Datenbank ausgelesen werden und versch. Variablen zugewiesen werden, global verfügbar zu machen.

Das ist nötig da ich diese Variablen dann mit andren Programmteilen weiterverarbeiten muss.

Das sieht in etwa so aus:

Code: Alles auswählen

def eingabe_hauptspeise(id):
    db = pyodbc.connect("dsn=MySQL_liste")
    cursor = db.cursor()
    if len(id) == 1:
        id = id[0]
        sql = ("""SELECT * FROM hauptspeisen WHERE id = '%s'"""% (id))
        global id_1
        global hauptspeise
        global brutto
        global netto
        global bestellung
        for row in cursor.execute(sql):
            id_1 = {}
            hauptspeise = row.hauptspeise
            brutto = row.brutto
            netto = row.netto
            id_1 = {"hauptspeise":hauptspeise, "brutto":brutto, "netto":netto}
            bestellung = id_1.values()      
            print bestellung[1], bestellung [0], bestellung [2]
            db.commit()

Wenn ich dieses Modul importiere, scheinen diese Variablen aber nicht global auf, d.h. NameError.

Designfehler?

[Zap]

Warum muss das ganze mit globalen Variablen umgesetzt werden!?
Es wäre doch sauberer wenn deine Funktion die Daten als Liste von Dictionaries
oder ähnliches zurück gibt.

Edit:
---------------
Allgemein zu deinem Code:
- Der Funktionsname "eingabe_hauptspeise" passt irgendwie nicht zu dem was die Funktion macht.
- "id" scheint ja irgendwie eine Sequenz von Werten zu sein, auch hier passt der Name nicht
- schmeisse alles was global ist raus und baue dir eine sinnvolle Struktur die du per return zurück geben kannst.
- ein db.commit() macht bei einer SELECT query wenig Sinn, ein close() wäre hingegen angebracht.

[ms4py]

- "id" scheint ja irgendwie eine Sequenz von Werten zu sein, auch hier passt der Name nicht

Außerdem überschreibt es ein Builtin...

[Hyperion]

Außerdem erscheint der Datenbank connect deplatziert. Zudem läuft er ohne das Abfangen von Exceptions ab. Ich würde den schon in eine connect_db()-Funktion auslagern. Vermutlich bleibt es ja nicht bei der einen Abfrage und damit kann man ihn dann wiederverwenden...

[EyDu]

Außerdem:

- Du solltest Anfragen nie wie in Zeile 7 zusammensetzen, die execute-Methode hat einen zweiten Parameter dafür. Andernfalls handelst du dir leicht die Möglichkeit für SQL-Injections ein.
- Zeile 14 ist überflüssig. Du musst vor der Verwendung nichts deklarieren.
- Mit einer vernünftigen Anfrage könntest du die Zeilen 15 bis 18 zu einer zusammenfassen.
- Ist dir bewusst, dass die Reihenfolge in Dictionaries nicht unbedingt beibehalten wird?
- Wenn du Indizes verwendest, gehört vor diese keine Leerzeichen. Diese beziehen sich immer auf den vorausgegangenen Ausdruck und stehen nicht für sich alleine.
- Du solltest über Konstanten für das Dictionary nachdenken.
- "eingabe_hauptspeise" ist ein sehr seltsamer Name für eine Funktion, wo doch nichts eingegeben wird.
- Was passiert, wenn len(id) nicht gleich 1 ist? Diesen Fall behandelst du gar nicht.

Vieles andere wurde ja schon gesagt.

[DasIch]

Außerdem überschreibt es ein Builtin...

Das nutzt du nicht ernsthaft als Argument? Ich überschreibe ständig builtins, solange man es nur lokal macht ist dass absolut kein Problem.

[m6T]

Danke mal für die Tipps und Verbesserungsvorschläge.

Die Funktion eingabe_Hauptspeise hat den Sinn dass eine ID des Essens (die ID's werden vorher über eine andere Funktion ausgegeben) der Funktion übergeben wird, und die Funktion das Essen über die ID ermittelt und den Variablen zuweist. Der Input geschieht im Hauptprogramm über einen input().

Die Funktionen kümmert sich auch um Fälle wenn len(id) länger als 1 ist, jedoch hab ich die anderen Teile weggelassen da sie im grunde gleich aufgebaut sind wie der Teil wenn len(id) = 1.

Ich werd mal sehen dass ich die Tipps umsetze und versuche das ganze über ein Dictionary zu regeln. Danke soweit!

[Zap]

Außerdem überschreibt es ein Builtin...

Das nutzt du nicht ernsthaft als Argument? Ich überschreibe ständig builtins, solange man es nur lokal macht ist dass absolut kein Problem.

Ich bin schon der Meinung das man einen Anfänger darauf hinweisen sollte.
Ihm muss zumindest bewusst sein das sowas ein paar Zeilen später ein selbst gelegter Stolperstein sein kann.

Ich überschreibe in der Regel keine builtins da erstens das Syntaxhighlighting der IDE irreführend ist und
zweitens seit geraumer Zeit pylint in meinen Projekten im Einsatz ist.
Und ich finde es gut das auch dieser einen darauf aufmerksam macht.

[Hyperion]

@DasIch: Ich glaube Dir natürlich, dass Du häufiger selber built-ins überschreibst. Aber glaubst Du denn auch, dass man das auch noch "empfehlen" sollte?

@m6T:
Was genau steht denn nun in einer "id"? Wieso kann len(id) > 1 sein? Wenn ja, wäre das ja ein Iterable und der Name "id" wäre auch noch unpassend.
Ist das eine GUI-Anwednung? Wenn nein, soltest Du unter Python < 3.x immer raw_input() statt input() nutzen.

[DasIch]

@DasIch: Ich glaube Dir natürlich, dass Du häufiger selber built-ins überschreibst. Aber glaubst Du denn auch, dass man das auch noch "empfehlen" sollte?

Ich empfehle builtins nicht um jeden Preis nicht zu überschreiben.

Das Ziel beim schreiben von Code sollte Lesbarkeit und Verständnis sein und wenn Regeln wie diese oder PEP8 dieses Ziel in einem bestimmten Kontext nicht erreichen muss ich Ausnahmen oder sogar neue Regeln schaffen.

Desweiteren erlaubt eine gute IDE dass Syntax Highlighting für bestimmte Dinge abzuschalten, ich lasse zum Beispiel builtins nicht highlighten. Wobei es natürlich besser wäre semantisches Highlighting zu haben dann stellt sich so ein Problem gar nicht erst.

[Hyperion]

Ich empfehle builtins nicht um jeden Preis nicht zu überschreiben.

Das Ziel beim schreiben von Code sollte Lesbarkeit und Verständnis sein und wenn Regeln wie diese oder PEP8 dieses Ziel in einem bestimmten Kontext nicht erreichen muss ich Ausnahmen oder sogar neue Regeln schaffen.

Hm... dann müßte "file" aber fast immer überschrieben werden. Und das Verständnis könnte durchaus darunter leiden, wenn man nicht das lokale Konstrukt auf den ersten Blick sieht, sondern eben das built-in.

[derdon]

Hm... dann müßte "file" aber fast immer überschrieben werden.

Für Dateinamen nutze ich file_name, für Dateiobjekte file_object oder einfach nur f. file allein ist mir zu mehrdeutig.

[jbs]

In PEP 8 steht, dass man Ausnahmen machen darf, wenn es der verständlichkeit dient.

Und ich meine an

Code: Alles auswählen

...
member[id]
...

sollte verständlich sein.

Zur Not geht ja noch: ``__builtin__.id``, was natürlich wieder unsauber ist.


P.S.: Ich will jetzt nicht zum Überschreiben von builtins aufrufen . Man sollte sie natürlich vermeiden, wenn es nicht klarer wird.

[DasIch]

Hm... dann müßte "file" aber fast immer überschrieben werden.

``file`` sollte man sowieso nicht benutzen es sei den man erbt davon.

[Leonidas]

Hm... dann müßte "file" aber fast immer überschrieben werden.

``file`` sollte man sowieso nicht benutzen es sei den man erbt davon.

Welche builtins überschreibst du denn üblicherweise? Weil ich überschreibe eigentlich nie welche, was auch daran liegt dass die Namen idR sowieso selten als gute Variablennamen taugen.

[DasIch]

Hm... dann müßte "file" aber fast immer überschrieben werden.

``file`` sollte man sowieso nicht benutzen es sei den man erbt davon.

Welche builtins überschreibst du denn üblicherweise?

``id`` und ``file`` als einfache Namen. ``all`` überschreibe ich manchmal mit einer Funktion die False zurück gibt wenn das iterable leer ist, wenn ich mich dabei erwische mehrmals ``all(foo) and foo`` zu schreiben.

EDIT: Fehler korrigiert, danke BlackJack

[EyDu]

Welche builtins überschreibst du denn üblicherweise?

``id`` und ``file`` als einfache Namen. ``all`` überschreibe ich manchmal mit einer Funktion die False zurück gibt wenn das iterable leer ist, wenn ich mich dabei erwische mehrmals ``all(foo) or foo`` zu schreiben.

"file" finde ich einen äußerst unglücklichen Namen aus. Dieser sagt genau nichts aus. Da würde ich lieber einen Namen wählen, welcher Aussagt, was in der Datei drin ist, oder was rein soll.
"all" mit einem anderen Verhalten zu überschreiben ist auch sehr unglücklich. Ich würde nicht erwarten, dass "all" auf einmal bei leeren Listen ein anderes Ergebnis liefert.

[BlackJack]

@DasIch: Bist Du sicher, dass Du nicht ``foo and all(foo)`` meintest!? Denn ``all(foo) or foo`` ergibt bei leerem `foo` auch `True`.

[DasIch]

"file" finde ich einen äußerst unglücklichen Namen aus. Dieser sagt genau nichts aus. Da würde ich lieber einen Namen wählen, welcher Aussagt, was in der Datei drin ist, oder was rein soll.

Wenn ich in ``Config.__init__`` eine Datei öffne und parse muss ich die nicht auch noch ``config_file`` nennen, dass ist redundant.

@Blackjack: Jetzt nicht mehr

[sma]

Um mal nach all diesem Gerede über Namen auf die eigentliche Frage zurückzukommen:

Designfehler?

Ja.

Das ganze sollte so aussehen:

Code: Alles auswählen

hauptspeise, brutto, netto = lies_hauptspeise("whatever")
...
def lies_hauptspeise(whatever):
    ...
    return row.hauptspeise, row.brutto, row.netto

Noch besser wäre, ein Objekt mit passenden Attribute zu benutzen.

Stefan