Verbesserungen für dieses (wahrscheinlich schlechte) script

[BlackJack]

@Kamik423: Wie kann man eine so einfache so Sache, so verdammt kaputt und kompliziert machen!? Die API ist eine Katastrophe, die ganzen Dateitests sind Mist, JSON kann auch mehr als eine Zeile enthalten, auch wenn *Du* das jetzt nicht so schreibst (an welcher Stelle ist das eigentlich garantiert?), und natürlich nutzt Du ``+`` um Zeichenketten zusammenzusetzen.

[Kamik423]

Hallo, und danke noch einmal an BlackJack für die sehr offene Antwort

Die API ist eine Katastrophe

Es gibt noch zwei dinge die ich generell anmerken muss, da ich sie wahrscheinlich nicht ausreichend ausgedrückt habe:
Der Code ist als unterprogramm gemeint um ein größeres script mit einer datei interfacen zu lassen. Deswegen würde es keine multiline jsons geben, und der code sieht nutzlos aus für eine anwendung durch menschen.
Und ich bin noch relativ neu zu PYTHON, Ich habe schon länger programmiert, kenne aber noch nicht alle möglichkeiten und module, die Python bietet, deswegen schreibe ich manchmal viel code, das dinge macht, die ein simpler import lösen könnte. Es ist großartig, dass ihr mich auf dies module hinweißt.

die ganzen Dateitests sind Mist

was meinst du mit Dateitests? die ganzen if-statements

JSON kann auch mehr als eine Zeile enthalten, auch wenn *Du* das jetzt nicht so schreibst (an welcher Stelle ist das eigentlich garantiert?)

der code ist nur ale kleiner teil eines grösseren Programms gemeint. wenn man nur den code benutzt um mit diner datei zu interfacen, dann gibt es keine multiline statements.

und natürlich nutzt Du ``+`` um Zeichenketten zusammenzusetzen.

da habe ich mich vertippt. und ich bin jetzt auch schon davon weg.

Sirius, meinst du so etwas mit dem abfangen?

Code: Alles auswählen

import os
import json

"""
most of the def's are created for a list with following format:

{"titleOne":valueOne,"titleTwo":valueTwo,"titleThree":valueThree}

however:
-createFile
-read
-write
should work for any json formatted data

createFile(path, filename)          creates a file with name FILENAME at PATH
read(filePath)                      returns the read out text from file PATH            must be valid json
write(filePath, data)               writes DATA in the file PATH                        must be valid json
titleList(data)                     returns a list with the titles in DATA. format:     see above
indexInList(data, title)            returns the index of TITLE in DATA. format:         see above
getValue(data, title)               returns the value of TITLE in DATA. format:         see above
removeFromList(data, title)         removes TITLE from DATA. format:                    see above
inList(data, title)                 returns True if TITLE is in DATA. format:           see above
setValue(data, title, value)        sets TITLE in DATA to VALUE. format:                see above
addToList(data, title, value)       adds TITLE with value VALUE to DATA. format:        see above
rename(data, title, newTitle)       renames TITLE in DATA to NEWTITLE. format:          see above

created by kamik423
"""


def createFile(path, filename): 
    filepath=os.path.join(path, filename)
    try:
        try:
            os.makedirs(path)
        with open(filepath, 'w') as VDfile:
            VDfile.write("{}")
            
def read(filePath):  
    try:
        with open(filePath, 'r') as VDfile:
            try:
                fileData=json.loads(VDfile.readline())
                return fileData
            except:
                return []
    except:
        return []

def write(filePath, data):
    try:
        with open(filePath, 'w') as VDfile:
            try:
                VDfile.write(json.dumps(data))

def titleList(data):
    titles = []
    try:
        dataItems = data.items()
        for i in dataItems:
            titles.append(i[0])
        return titles
    except:
        return []

def indexInList(data, title):
    listOfTitles = titleList(data)
    try:
        return listOfTitles.index(title)
    else:
        return None

def getValue(data, title):
    titleIndex = indexInList(data, title)
    try:
        value = data.items()[titleIndex][1]
        return value
    else:
        return -1

def removeFromList(data, title):
    try:
        titleIndex = indexInList(data, title)
        value = getValue(data,title)
        dictValue = {title:value}
        data.pop(title)
    return data

def inList(data, title):
    return title in titleList(data)

def setValue(data, title, value):
    try:
        print data[title]
        data[title]=value
    return data

def addToList(data, title, value):
    data[title]=value
    return data

def rename(data, title, newTitle):
    if inList(data, title):
        value = getValue(data,title)
        data = removeFromList(data,title)
        data = addToList(data, str(newTitle), value)
    return data

[Sirius3]

@Kamik423: try ohne except ist syntaktisch falsch. except ohne Angabe einer Exceptionklasse ist schlechter Stil. json.dumps garantiert nicht, dass alles in einer Zeile geschrieben wird. Deshalb kannst Du auch nicht sagen, dass Du nur eine Zeile schreibst. Was ist so schlimm daran, readline durch read zu ersetzen? Dann ist die ganze API immer noch viel zu kompliziert. titleList kann man z.B. durch data.keys() ersetzen. Ein Wörterbuch ist ja gerade dazu da, dass man den Key als Index benutzt und sich nicht einen Zahlenwertindex erst suchen muß. Damit wird getValue zu data.get(title, -1), wobei zu klären ist, ob -1 ein guter Defaultwert ist. In removeFromList werden 3 von 3 Variablen nicht verwendet.
Es ist auch egal, ob die Funktionen die nach außen sichtbare API sind oder nur intern verwendet werden. Sinnvoll sollten sie auf jeden Fall sein.

[Kamik423]

Ok, danke an Sirius, für das klare darlegen meiner Fehler. Ich habe die dict documentation noch einmal durchgelesen, und noch so einiges in meinem code verbessert. Vielen dank.

was sind noch andere probleme mit der API?

Code: Alles auswählen

import os
import json

"""
most of the def's are created for a list with following format:

{"titleOne":valueOne,"titleTwo":valueTwo,"titleThree":valueThree}

however:
-createFile
-read
-write
should work for any json formatted data

createFile(path, filename)          creates a file with name FILENAME at PATH
read(filePath)                      returns the read out text from file PATH            must be valid json
write(filePath, data)               writes DATA in the file PATH                        must be valid json
titleList(data)                     returns a list with the titles in DATA. format:     see above
indexInList(data, title)            returns the index of TITLE in DATA. format:         see above
removeFromList(data, title)         removes TITLE from DATA. format:                    see above
inList(data, title)                 returns True if TITLE is in DATA. format:           see above
setValue(data, title, value)        sets TITLE in DATA to VALUE. format:                see above
addToList(data, title, value)       adds TITLE with value VALUE to DATA. format:        see above
rename(data, title, newTitle)       renames TITLE in DATA to NEWTITLE. format:          see above

created by kamik423
"""


def createFile(path, filename): 
    filepath=os.path.join(path, filename)
    try:
        if not os.path.exists(path):
            os.makedirs(path)
        
        try:
            with open(filepath, 'w') as VDfile:
                VDfile.write("{}")
        except Exception, e:
            print str(e)
    except Exception, e:
        print str(e)
            
def read(filePath):  
    try:
        with open(filePath, 'r') as VDfile:
            try:
                fileData=json.loads(VDfile.read())
                return fileData
            except Exception, e:
                print str(e)
                return []
    except Exception, e:
        print str(e)
        return []

def write(filePath, data):
    try:
        with open(filePath, 'w') as VDfile:
            try:
                VDfile.write(json.dumps(data))
            except Exception, e:
                print str(e)
    except Exception, e:
        print str(e)

def titleList(data):
    try:
        return data.keys()
    except Exception, e:
        print str(e)
        return []

def indexInList(data, title):
    listOfTitles = titleList(data)
    try:
        return listOfTitles.index(title)
    except Exception, e:
        print str(e)
        return None

def removeFromList(data, title):
    try:
        del data[title]
    except Exception, e:
        print str(e)
    return data

def inList(data, title):
    return data.has_key(title)

def setValue(data, title, value):
    try:
        data[title]=value
    except Exception, e:
        print str(e)
    return data

def addToList(data, title, value):
    data[title]=value
    return data

def rename(data, title, newTitle):
    try:
        value = getValue(data,title)
        data = removeFromList(data,title)
        data = addToList(data, str(newTitle), value)
    except Exception, e:
        print str(e)
    return data

[EyDu]

Das ist noch immer keine sinnvolle Behandlung von Ausnahmen. In try-Blöcken sollte so wenig wie möglich stehen und die except-Blöcke dürfen auch nur die Ausnahmen bearbeiten, mit denen sie auch umgehen können. Ob du nun alle Exceptions abfängst und alle von "Exception" erbenden ist fast egal. Du fängst noch immer viel zu viele ab.

Bei dir es es sogar noch viel schlimmer: Du fängst Ausnahmen ab und versetzt damit dein Programm in einen ungültigen Zustand. Das passiert zum Beispiel bei der rename-Funktion. Schlägt der Aufruf von getValue fehl, dann wird "data" niemals definiert. Du behandelst, gibst den Fehler aus und versuchst dann "data" zurückzugeben. Dann kracht es hier. Ganz offensichtlich kannst du dort eine Ausnahme also nicht sinnvoll behandeln. Wenn du es nicht kannst, dann lasse es. Dann bekommst du wenigstens dort eine Ausnahme, wo der Fehler auftritt und nicht erst einige Zeilen später.

Oder noch schlimmer: Der Aufruf von addToList schlägt fehl. Dann ist das Ergebnis von removeFromList an data gebunden, die Ausnahme wird behandelt und ein falscher Wert zurückgegeben. Der landet dann irgendwo in deinem Programm, welches einfach munter weiterläuft. Versuche mal solche Fehler zu finden. Ganz blöd wird es natürlich, so wie in deinem Fall, wenn man auf Dateien operiert. Du kannst dir ja ausmalen was passiert, wenn du falsche Dateien löscht überschreibst oder verschiebst.

[BlackJack]

@Kamik423: Die ganzen ``except Exception`` sollten überhaupt gar nicht sein. Wenn man die Ausnahme nicht sinnvoll behandeln kann, dann sollte man sie halt einfach gar nicht behandeln. Und einfach mit ``print`` etwas ausgeben ist keine sinnvolle behandlung. Davon bekommt der Aufrufer ja nichts mit. Gerde Code der als Bibliothek verwendet werden soll, hat keine Ausgaben zu machen. Höchstens in Form von Logging, aber auch dann darf der Probleme nicht einfach so vor dem Aufrufer verstecken. Du ersetzt mit Deiner „Ausnahmebehandlung” die Ausnahmen durch spezielle Fehlerwerte — etwas was man durch Ausnahmen ja gerade loswerden will, weils blöd ist dauernd irgendwelche Rückgaberwerte prüfen zu müssen. Letztlich bleibt dann von Deinen Funktionen nicht genug Substanz die die Funktionen rechtfertigen würden. Da kann man auch gleich Wörterbücher und das `json`-Modul verwenden. Dieser ganze Code ist einfach nur überflüssig.

[Kamik423]

ok,
da ich jetzt mehr über dict und json weiß ist der großteil des scripts überflüssig.
Ich muss mich entschuldigen, da ihr versucht habt mir das von anfang an zu sagen, und ich nur zu blöd war es zu verstehen.
Vielen dank nochmal an alle die versucht haben mir idioten etwas beizubringen.
THE END