Funktionen/Listen übergeben - py3

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
Hjokir
User
Beiträge: 1
Registriert: Donnerstag 13. Oktober 2011, 20:19

Sodele, bevor ich Anfange von meinen Sorgen zu schreiben... ich bin neu hier, guten Tag allesamt :)

Hier jetzt zum Problem, ich habe eine Funktion geschrieben die mir Zeilen ausliest und die Werte dann in eine Liste schreibt, soweit so gut. Funktioniert auch bis dato wunderbar. Allerdings komm ich nun dazu das ich mehrere Listen haben, die Funktion dazu allerdings immer diesselbe wäre, nur das der Listenname sich ändert. Optimal wäre quasi wenn ich den Listennamen als Parameter der funktion übergeben könnte und der das dann entsprechend irgendwie auch annimmt.

Hier mal der Quellcode, ist übrigens um eine Konfiguration für ein vmstat tool auszulesen gedacht (halt erstmal was simples), danach sollen die Listen in eine Datei geschrieben werden und von einem anderen Programm entsprechend benutzt werden

Code: Alles auswählen

#!/usr/bin/python3
import sys
import re

# Config
conf = "example.conf"

def ASG_VAL_INS(i):
    j = 1
    for x in range(len(newline)):
        asg_cpu[i][j-1] = int(newline[j])
        j = j+1

def ASG_SYS(name, pos):
    if newline[0] == name:
        asg_sys[pos] = 1
        ASG_VAL_INS(pos+1)
    else:
        trigger = 0

def ASG_CPU(name, pos):
    if newline[0] == name:
        asg_cpu[pos] = 1
        ASG_VAL_INS(pos+1)
    else:
        trigger = 0

# CPU      act  us    v1 v2    sy     v1 v2    id     v1 v2    wa     v1 v2    st     v1 v2
asg_cpu = [ 0 , 0 , [ 0 , 0 ] , 0 , [ 0 , 0 ] , 0 , [ 0 , 0 ] , 0 , [ 0 , 0 ] , 0 , [ 0 , 0 ] ]


try:
    obj = open(conf, "r")
except:
    # ErrorLog
    print("File", conf, "not found")
    sys.exit(1)


trigger = 0
for line in obj:
    newline = re.findall(r'\w+', line)

    try:
        # SYSTEM
        if trigger == 5:
            ASG_SYS("interrupts", 1)
            ASG_SYS("changes", 3)
        
        if newline[0] == "system":
            asg_sys = 1

            trigger = 5

        # CPU  
        if trigger == 6:
            ASG_CPU("timeuser", 1)
            ASG_CPU("timekernel", 3)
            ASG_CPU("idle", 5)
            ASG_CPU("waiting", 7)
            ASG_CPU("virtuel", 9)                

        if newline[0] == "cpu":
            asg_cpu[0] = 1
    
            trigger = 6
    except:
        continue        

print("CPU:", asg_cpu)

obj.close()    
Die example.conf sieht derzeit nur so aus, desweiteren würde oben nun die _VAL_INS für Sys auch fehlen, den ganzen Kram müsste ich halt für jede Liste einmal bauen.

Code: Alles auswählen

[cpu]
waiting=12,34
idle=47,11

etc...
bla
[system]
something here
and so on
BlackJack

@Hjokir: Als erstes mal: Für solche Konfigurationsdateien gibt es `ConfigParser` in der Standardbibliothek.

Dann ist das übergeben eines Namens keine gute Idee, wenn das zu so Fragen führt wie lege ich dynamisch eine Variable mit einem Namen an, der in einer Variable steht. So etwas macht man nicht. Dafür gibt es Wörterbücher (`dict`).

Du solltest mal einen Blick in PEP 8 -- Style Guide for Python Code werfen und Deine Namen anpassen. Komplett Grossbuchstaben sind eigentlich für Konstanten vorgesehen und nicht für Funktionsnamen. Ausserdem sind die Funktionsnamen ziemlich kryptisch. Die könnten deutlich besser sein.

Funktionen sollten nicht einfach so auf Daten zugreifen die nicht als Argument in die Funktion hinein gekommen sind. `ASG_VAL_INS()` greift auf `newline` und `asg_cpu` zu ohne dass das aus der Funktionssignatur ersichtlich wird. Das ist ziemlich unübersichtlich.

In der Funktion ist eine Schleife in der an `x` die Zahlen von 0 bis ``len(newline) - 1`` gebunden werden, aber die benutzt Du gar nicht, sondern zählst stattdessen `j` selber noch einmal ”parallel” hoch. Wobei ``for i in range(len(obj))`` in Python sowieso ein „Anti-Pattern“ ist. Ebenso normalerweise das vorbelegen von Listen mit Ersatzwerten um sie dann über einen laufenden Index später mit den richtigen Werten zu überschreiben. Es ist nicht unverhältnismässig teuer einfach eine neue Liste zu erstellen, welche die alte ersetzt.

Du speicherst anscheinend in den "us", "sy" 0en und 1en je nach dem ob ein Eintrag dafür gefunden wird? Python hat Wahrheitswerte `True` und `False` für solche Fälle. Das ist deutlicher, weil der Leser dann weiss, dass es nur zwei Werte gibt, und nicht beliebige Zahlen und das damit nicht gerechnet werden soll.

Die Zuweisungen an `trigger` in `ASG_SYS()` und `ASG_CPU()` bewirken nichts, weil das hier ein lokaler Name ist, der nur innerhalb der Funktion sichtbar ist. Namen ausserhalb einer Funktion neu zu binden ist keine gute Idee. Damit handelt man sich genau die gleichen unübersichtlichen Abhängigkeiten ein, wie bei Zugriff auf Datenstrukturen, die nicht als Argument übergeben werden.

Die `asg_cpu`-Struktur sieht gruselig aus. Mach daraus ein Wörterbuch oder eine Klasse, dann musst Du nicht als Kommantar darüber schreiben was an welcher Stelle liegt. Magische Indexe führen zu schwer verständlichen Programmen. Bei einem Zugriff auf ``asg_cpu[7]`` muss man anfangen zu zählen und im Kommentar über der ”Definition” nachsehen was das eigentlich bedeutet. Ein ``asg_cpu['waiting'] `` oder ``asg_cpu.waiting`` hat dieses Problem nicht. Ausserdem ist die Liste recht unregelmässig aufgebaut. Man versucht normalerweise „gleiche“ Dinge in einer Liste zu speichern. Also Dinge über die man eine Schleife schreiben kann, und wo jedes Objekt gleich behandelt werden kann.
Antworten