Parameterübergabe bei Funktionsaufruf

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
CarlosDelgado
User
Beiträge: 5
Registriert: Montag 17. Januar 2022, 13:19

Hallo Forum

Ich habe eine Sache die ich nicht verstehe, die Übergabe von Listen bzw Arrays als Parameter.

Zu den Details:
Ich habe eine Klasse employees, mit den üblichen Eigenschaftten, Name, Vorname, Geburtstagdatum und paar andere. Davon hab ich natürlich einige. Alle
gesammelt in einem Array Mtarbeiterliste

Code: Alles auswählen

Mitarbeiterliste = []

class employee:
    counter = 0
    def __init__(self,firstname,lastname,gender,birthdate,jobtitle,street,zipcode,city,entrydate,leavedate,payroll,handicapped,department,personalnumber,costCenterNr,entity,costCenterName):
        employee.counter+=1
        self.id = employee.counter
        self.firstname = firstname
        self.lastname = lastname
        self.gender = gender

Das Problem macht folgende Funktion. Wenn ich

Code: Alles auswählen

def isNotInList(firstname,lastname,liste):
    NotInList = -1
    for obj in liste:
        #print(obj.firstname)
        if ((str(obj.firstname)==str(firstname)) and(str(obj.lastname) == str(lastname))):
            NotInList = obj.id
    return NotInList

Wenn ich diese Funktion aufrufe

Code: Alles auswählen

Nummer = isNotInList(Max,Müller,Mitarbeiterliste)
Dann wird Max nie gefunden, weil liste einfach immer leer ist.

der genaue Verlauf ist

main()
getImport(csvFile,Mitarbeilterliste)

und eben innerhalb getImport wird isNotInList aufgerufen. Innerhalb von getImport ist die Mitarbeiterliste noch gefüllt.

Wo sollte ich ansetzen mit dem suchen?
Habe ich etwas nicht ausreichend beschrieben?

Danke
Carlos
Sirius3
User
Beiträge: 17738
Registriert: Sonntag 21. Oktober 2012, 17:20

Das was Du hier postest, ist kein Python, sondern nur entfernt damit verwandt. Um Dir wirklich helfen zu können, mußt Du schon den exakten Code hier posten, und zwar alles, was für die Frage relevant ist, insbesondere die getImport-Funktion.

Variablennamen und Funktionen werden nach Konvention komplett klein geschrieben. Klassen dagegen mit großem Anfangsbuchstaben `Employee`.
Globale Klassenvariablen benutzt man nicht, es gibt auch kein reales Szenario, wo `counter` irgendetwas sinnvolles enthält.

`firstname` und `lastname` sind hoffentlich schon Strings, ein nochmaliges Konvertieren daher unsinnig.
`liste` und `obj` sind schlechte, weil zu generische, Variablennamen, `employees` und `employee` wären besser.
Die Funktion `is_not_in_list` macht ja das genaue Gegenteil, was der Funktionsname erwarten läßt. Man benutzt keine magischen Zahlen, `None` wäre der übliche Wert für "nicht vorhanden".
CarlosDelgado
User
Beiträge: 5
Registriert: Montag 17. Januar 2022, 13:19

Sirius3 hat geschrieben: Montag 17. Januar 2022, 14:18 Das was Du hier postest, ist kein Python, sondern nur entfernt damit verwandt.
Wow.
Ich habe ja erwartet, dass man als Anfänger nicht mit Samthandschuhen angefasst wird, aber noch arroganter hätte ich kaum antworten können. Respekt.
Ob die Funktion isNotInList ein None zurückliefert oder einen Integer ist für die Fragestellung unerheblich und mir ist natürlich klar, dass beim nächsten Refaktoring
das dran kommen muss. Danke auch für den Hinweis der Namenskonvention. Meinst Du die Liste ist damit korrekt übergeben?

Carlos
CarlosDelgado
User
Beiträge: 5
Registriert: Montag 17. Januar 2022, 13:19

Code: Alles auswählen

def getImport(ifile,liste):
    new_employees = 0
    just_updated_employees = 0
    lid,lfirstname,llastname,lgender,lbirthdate,ljobtitle,lzipcode,lstreet,lcity,lentrydate,lleavedate,lpayroll,lhandicapped,ldepartment,lpersonalnumber,lcostCenterNr,lentity,lcostCenterName = ifile.order
    readFile = str(ifile.name)
    with open(readFile) as csv_file:
        csv_reader = csv.reader(csv_file, delimiter=';')
        line_count = 0
        for row in csv_reader:
            if line_count < ifile.offset:
                line_count += 1
            else:
                index = isNotInList(row[lfirstname],row[llastname],liste)
                if ( index == -1 ):
                    NewEmp = employeeFromRow(row,ifile.order,ifile.time)
                    Mitarbeiterliste.append(NewEmp)
                    writeObj2file(NewEmp,DiffFile)
                    new_employees +=1
                else:
                    updateEmployee(row,index-1,Mitarbeiterliste,ifile)
                    just_updated_employees +=1
                line_count += 1
        print('Imported: ' + str(line_count) + ' lines from ' + str(ifile.name))
        print ("New employees: " + str(new_employees))
        print ("Updated employees:" + str(just_updated_employees))
Sirius3
User
Beiträge: 17738
Registriert: Sonntag 21. Oktober 2012, 17:20

Entschuldige, falls das hart klingt, aber Computer sind sehr pingelig, was das anbelangt, und auch beim Stellen von Fragen ist es wichtig, möglichst exakt zu sein.

Was ist denn `ifile` für ein Objekt? Warum hat das so viele Attribute, aber Du mußt trotzdem selbst einen csv.reader erzeugen um den Inhalt zu lesen?
Viele Funktionen scheinen implizit Objekte zu verändern, ohne dass man das beim Aufruf sehen kann.
Wie sieht z.B. `updateEmployee` aus? Ich vermute, dass das irgendwie `Mitarbeiterliste` verändert, aber an anderer Stelle benutzt Du einfach `Mitarbeiterliste.append`.
`Mitarbeiterliste` kommt aus dem Nichts, das sollte nicht sein, alles was eine Funktion braucht, muß sie auch über ihre Argumente bekommen.
Dann gibt es `liste`, was Du in isNotInList verwendest, was aber wahrscheinlich `Mitarbeiterliste` sein sollte, oder alle `Mitarbeiterliste` sollten eigentlich `liste` sein?
Benutzeravatar
__blackjack__
User
Beiträge: 13071
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@CarlosDelgado: Das mag hart klingen, es sieht aber halt wirklich nicht aus wie Python sondern eher wie BASIC oder Pascal. Und das ist halt kein idiomatisches Python was man so schreiben würde wenn man wirklich in Python programmiert.

Bezüglich Begriffen: Listen sind keine Arrays. Es gibt auch Arrays, und damit meint man bei Python üblicherweise die Array-Datentypen aus dem Numpy-Package.

`Mitarbeiterliste` ist eine globale Variable → die darf es nicht geben. Und Grunddatentypen sollten nicht in Namen stehen, denn den Typ ändert man während der Entwicklung gar nicht so selten, oder es funktionieren auch andere Typen die sich gleich verhalten und dann hat man entweder verwirrende Namen im Quelltext stehen, oder man muss alle betroffenen Namen suchen und ändern, was unnötig Arbeit macht.

`isNotInList()` hat den falschen Namen oder macht das falsche. Wenn der Name stimmt, dann würde die Funktion `True` oder `False` zurückgeben. Wenn der Rückgabwert, die ID oder -1 stimmt, dann ist der Name falsch. Gehen wir mal davon aus, dass der Name falsch ist und der Rückgabwert richtig ist, dann ist der Name `index` an den das Ergebnis gebunden wird falsch, oder es ist der Rückgabewert falsch. Denn die ID ist kein Index. Es wird aber als Index in die Liste verwendet, was nur funktioniert wenn die ID immer dem Index entspricht. Was a) nicht wirklich garantiert ist, und b) die ID redundant machen würde, wenn Index und ID immer den gleichen Wert hätten.

Ich gehe mal davon aus, dass hier der Index gemeint ist, weil der Code der den Wert verwendet nur dann wirklich robust ist. Dann heisst `isNotInList()` eigentlich `get_index()`. Und statt einer magischen Zahl oder `None` sollte eine Ausnahme ausgelöst werden wenn kein Index gefunden werden kann.

`getImport()` ist als Name falsch. Da wird kein Import geholt, da wird ein Import durchgeführt.

Was bedeutet das `i` bei `ifile`? Kryptische Abkürzungen in Namen sollte man vermeiden.

`ifile` scheint kein Python-Datei(ähnliches) Objekt zu sein. Da ist `file` als Name ein bisschen verwirrend. Das scheinen eher Metainformationen *über* eine Datei zu sein‽

Was sollen die `l`-Präfixe bei den Einzelwerten von `file.order` bedeuten?

Von den ganzen Namen werden auch nur zwei tatsächlich verwendet, dann braucht man die nicht *alle* an Namen binden.

`readFile` ist als Name für einen Dateinamen falsch. Das klingt entweder nach einer Funktion die eine Datei liest, oder nach einer gelesenen Datei wenn man `read` als Vergangenheitsform betrachtet. Letztlich braucht man den Dateinamen aber auch gar nicht an einen zusätzlichen Namen binden der nur einmal und gleich in der nächsten Zeile verwendet wird. Und eine Zeichenkette ist das hoffentlich schon.

CSV-Dateien müssen für das `cvs`-Modul mit ``newline=""`` als Argument geöffnet werden — siehe Dokumentation. Und bei Textdateien sollte man immer explizit die Kodierung angeben, sonst kann man unangenehme Überraschungen erleben.

`line_count` wird sowohl im ``if``- als auch im ``else``-Zweig um 1 erhöht, also eigentlich immer, das kann also aus den beiden Zweigen raus und entweder vor oder hinter das ``if``/``else``-Konstrukt. Dann ist das ``if`` leer, also überflüssig. Und statt das manuell hoch zu zählen kann man das dann auch einfach mit `enumerate()` generieren.

Statt *in* der Schleife für jeden Datensatz zu prüfen ob der unter den anfänglich zu ignorierenden ist, würde man besser *vor* der Schleife die entsprechende Anzahl von Datensätzen überlesen.

`line_count` würde ich auch `row_count` nennen weil man dann besser sieht, dass das zu `row` gehört. Und im englischen ist auch ein Unterschied zwischen „line“ und „row“, und bei CSV-Dateien gilt nicht zwangsläufig „line“ (Zeile) = „row“ (Datensatz), denn ein Datensatz kann sich durchaus auch über mehr als eine Zeile erstrecken wenn man die Datei allgemein als Textdatei betrachtet.

`new_employees` kann man sich im Grunde sparen, denn das ist ja die Länge der lokalen `mitarbeiter`-Liste. *Die* sollte `new_employees` heissen, damit man da nicht so ein komisches Denglisch in den Namen hat.

Und am Ende sollte man dann `mitarbeiter` zurückgeben, denn sonst macht das ja keinen Sinn da eine Liste für zu füllen die nirgends verwendet.

Die Funktion verwendet `DiffFile` aus dem nichts, das muss ein Argument sein. Ausser es ist ein Dateiname und eine Konstante, dann ist es aber falsch benannt (Datei ≠ Dateiname), und falsch geschrieben weil Konstanten KOMPLETT_GROSS geschrieben werden, damit man sie als solche erkennen kann.

Zeichenketten und Werte mit `str()` und ``+`` zusammenstückeln ist eher BASIC denn Python. In Python gibt es dafür die `format()`-Methode auf Zeichenketten und f-Zeichenkettenliterale.

Ungetestet:

Code: Alles auswählen

from itertools import islice


def get_index(firstname, lastname, employees):
    try:
        return next(
            index
            for index, employee in enumerate(employees)
            if (employee.firstname, employee.lastname) == (firstname, lastname)
        )
    except StopIteration:
        raise ValueError("no employee found")


def import_employees(file_info, employees, diff_file):
    new_employees = []
    _, firstname_key, lastname_key, *_ = file_info.order
    with open(file_info.name, encoding="utf-8", newline="") as csv_file:
        numbered_rows = enumerate(csv.reader(csv_file, delimiter=";"), 1)
        updated_employees_count = 0
        row_count = 0
        for row_count, row in islice(numbered_rows, file_info.offset, None):
            try:
                index = get_index(
                    row[firstname_key], row[lastname_key], employees
                )
            except ValueError:
                employee = create_employee_from_row(
                    row, file_info.order, file_info.time
                )
                new_employees.append(employee)
                write_object_to_file(employee, diff_file)
            else:
                update_employee(row, index, len(new_employees), file_info)
                updated_employees_count += 1
        
        print(f"Imported: {row_count} lines from {file_info.name}")
        print(f"New employees: {len(new_employees)}")
        print(f"Updated employees: {updated_employees_count}")
        return new_employees
„All religions are the same: religion is basically guilt, with different holidays.” — Cathy Ladman
CarlosDelgado
User
Beiträge: 5
Registriert: Montag 17. Januar 2022, 13:19

Sirius3 hat geschrieben: Montag 17. Januar 2022, 15:47 Entschuldige, falls das hart klingt, aber Computer sind sehr pingelig, was das anbelangt, und auch beim Stellen von Fragen ist es wichtig, möglichst exakt zu sein.
Meine Frage umriss die Übergabe eines Objekts bzw Liste. Du hast alles an meinem Code moniert, nur die Frage nach der Liste offengelassen.
Ich anerkenne, dass es auch darum geht lesbaren Code zu schreiben, aber es war ein vernichtendes Urteil. Du musst wissen, ob Du einem Anfänger so motivierst oder nicht.
Schwamm drüber.
Was ist denn `ifile` für ein Objekt? Warum hat das so viele Attribute, aber Du mußt trotzdem selbst einen csv.reader erzeugen um den Inhalt zu lesen?
Ich lese csv Files ein. Aber mit ungefähr 5 oder 6 verschiedenen Formaten. Allen gemeinsam ist die Spalte firstname und lastname. Aber die ist einmal an Nullter Stelle
mal an dritter. Mal ist gender angegeben an 3 Stelle mal gar nicht.

lid,lfirstname,llastname,lgender, ...... = ifile.order

und ifile.oder könnte dann so aussehen [99,1,0,2,3,99,7,8,9,99,4,5,99,99,99,99,99,99]
99er werden also ignoriert, die anderen Zahlen sagen mir an welcher Stelle im csv File die Spalte ist.
Viele Funktionen scheinen implizit Objekte zu verändern, ohne dass man das beim Aufruf sehen kann.
Ja. Ist hysterisch gewachsen. Am Anfang hatte ich nur eine Funktion entwickelt, später drumherumgestrickt. Ist so ein mittelding zwischen: "Will ich diese Funktion refactoren" oder "wich ich die neue Funktion anfangen, die mir im Moment wichtiger erscheint". Wenn das ein No Go ist und so verstehe ich eure Kommentare, dann werde ich das sauber trennen.
Wie sieht z.B. `updateEmployee` aus? Ich vermute, dass das irgendwie `Mitarbeiterliste` verändert, aber an anderer Stelle benutzt Du einfach `Mitarbeiterliste.append`.
`Mitarbeiterliste` kommt aus dem Nichts, das sollte nicht sein, alles was eine Funktion braucht, muß sie auch über ihre Argumente bekommen.
Okay, ist notiert.
Dann gibt es `liste`, was Du in isNotInList verwendest, was aber wahrscheinlich `Mitarbeiterliste` sein sollte, oder alle `Mitarbeiterliste` sollten eigentlich `liste` sein?
isNotinList hatte anfangs tatsächlich einen Boolen als Rückgabewert. Später hab ich gemerkt, dass ich auch den Index benötige und diesen bequem auch von dieser Funktion bekommen kann.
Der index fängt bei Null an zu zählen. die ID bei 1. Deswegen habe ich beide.

Falls es in meinem obigen Text nicht deutlich gewordn ist, ich plane alle Eure Kritikpunkte zu übernehmen. Ich schreibe ja nichts in ein Anfängerforum und frage um Rat, wenn ich den
dann nicht beherzigen wollte. Auch die BASIC oder PASCAL Anmerkungen sind absolut zutreffend.
CarlosDelgado
User
Beiträge: 5
Registriert: Montag 17. Januar 2022, 13:19

__blackjack__ hat geschrieben: Montag 17. Januar 2022, 16:08 @CarlosDelgado: Das mag hart klingen, es sieht aber halt wirklich nicht aus wie Python sondern eher wie BASIC oder Pascal.
Nein, jetzt verstehe ich besser was gemeint ist. Und Pascal finde ich als Urteil überhaupt nicht beleidigend.

Ich werde alle Deine Anmerkungen sukzessive einarbeiten. Deine Mühe ist gut angelegt, ich werde Deinen Text mehrfach lesen.
Ich antworte Dir nicht auf alles, weil ich möchte, dass Du schnell eine Rückmeldung bekommst. Leider kann man Posts nicht mehr
korrigieren. Ich werde also in den nächsten tagen in diesem Thread wieder antworten.

Danke
Carlos
Benutzeravatar
__blackjack__
User
Beiträge: 13071
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

BASIC sollte auch nicht beleidigend oder abwertend sein. Beide Sprachen haben einen festen Platz in meinem Programmiererherzen. 🙂 BASIC war die erste Sprache bei mir, und Pascal unter MS-DOS meine Lieblingssprache. Das mit dem `str()` und ``+`` (oder bei Ausgaben ``;`` (oder bei späteren Microsoft-BASIC-Dialekten ``&``)) und die Klammersetzung bei einer Bedingung mit ``and``, das in Verbindung mit eher rein prozeduralem Code, und auch mal globalen Werten/Datenstrukturen, erinnert halt sehr an BASIC und teilweise Pascal.
„All religions are the same: religion is basically guilt, with different holidays.” — Cathy Ladman
Antworten