Erstellen einer Filmdatenbank

[gofid]

Hallo erstmal,

ich bin gerade dabei Python als meine erste Programmiersprache zu lernen und bin noch dementsprechend unbeholfen und hoffe ihr könnt mir ein paar Tipps und Verbesserungsvorschläge geben. Mein erstes Projekt soll ein einfaches Management Programm für eine Filmdatenbank werden. Ich hab mir das so vorgestellt das man natürlich zuerst die Daten eingeben kann (später vielleicht über eine GUI) und diese dann in eine Datenbank oder gepickelt speichert. Hinterher soll es dann möglich sein über Suchbegriffe die Filme zu finden. z.B. alle Filme mit Johnny Depp oder aus dem Jahr 1986 etc.
Bisher bin ich nur soweit das man die Eckdaten des Films in ein Dictionary speichert. Das funktioniert soweit ganz gut, allerdings bin ich mir sicher das an der ein oder anderen Stelle noch Verbesserungen möglich oder sogar nötig sind.
Danke schon mal im vorraus für jegliche Tipps.
Ach ja, ich benutze übrigens Python3.2

Hier ist mein bisheriger Code

Code: Alles auswählen

# Abfrage ob weitere Werte eingefügt werden sollen
def abfrage():
        while True:
                eingabe = input('Weiteren Datensatz hinzufügen? j/n \n')
                if eingabe == 'n':
                        return False
                elif eingabe == 'j':
                        return True
                else:
                        print('Geben Sie j oder n an')
                        continue

# Erstellen einer Liste für mehrere Values in einem Key
def multi_werte():
        temp_liste = []
        z = 0
        while True:
                temp_liste.insert(z, input(kategorie + ': '))
                if abfrage() == False:
                        return temp_liste
                        return False
                z = z + 1

# Dictionary für die Filmdaten erstellen
filmdaten = {'Titel':'', 'Regiesseur':'', 'Jahr':'', 'Bewertung':'', \
               'Genre':'', 'Schauspieler':''}

print('Geben Sie folgende Filmdaten ein:')

# Programm zum einfügen der Werte ins Dictionary
for kategorie in filmdaten.keys():
        
        if kategorie == 'Genre':
                temp_ausgabe = multi_werte()
                filmdaten[kategorie] = temp_ausgabe
                
        elif kategorie == 'Schauspieler':
                temp_ausgabe = multi_werte()
                filmdaten[kategorie] = temp_ausgabe

        elif kategorie == 'Jahr':
                while True:
                        try:
                                filmdaten[kategorie] = \
                                int(input(kategorie + ': '))
                                break
                        except:
                                print('Sie müssen eine Zahl eingeben')
        else:
                filmdaten[kategorie] = input(kategorie + ': ')

print(filmdaten)

Gruss
Gofid

[BlackJack]

@gofid: Die Kommentare über den Funktionen würden sich als DocStrings eignen.

`abfrage()` ist ein sehr allgemeiner Name. Da lässt sich sicher etwas Treffenderes finden. Auf der anderen Seite könnte man die Funktion vielleicht auch etwas allgemeiner machen, in dem zum Beispiel die Frage nicht fest kodiert wird, sondern als Argument übergeben werden kann. Die ``continue``-Anweisung am Ende der Schleife ist unnötig.

`multi_werte()` ist ein noch schlechterer Name weil man da überhaupt keine Idee hat was die Funktion eigentlich macht.

Datentypen sollte man in Namen vermeiden. Die sollten die Bedeutung des Objektes im Programm beschreiben und nicht den Typ. (Ausser natürlich bei Klassennamen, die ja einen Datentyp repräsentieren.) Das `_liste` bei `temp_liste` sollte man also weglassen. Bei Containertypen hat sich bei der Namensgebung die Mehrzahl bewährt. Statt dem nichtssagenden `temp` könnte man also zum Beispiel `werte` für `temp_liste` verwenden.

`z` und die `insert()`-Methode sind unnötig umständlich. Die Elemente werden ja immer an die letzte Position „eingefügt” also semantisch gesehen *angehängt*. Dafür gibt es eine Methode.

Wahrheitswerte sollte man nicht mit literalen Wahrheitswerten vergleichen. Denn da kommt nur wieder ein Wahrheitswert heraus. Wenn man auf ``== False`` oder ``!= True`` testen möchte, sollte man den Wert den man bereits hat einfach mit ``not`` negieren. Also ``if not abfrage():`` statt ``if abfrage() == False:``.

Zwei direkt aufeinander folgende ``return``-Anweisungen sind Unsinn. Die erste beendet den Programmfluss innerhalb der Funktion, so dass Code der danach steht, niemals ausgeführt werden kann.

Innerhalb von Funktionen sollte man auf nichts zugreifen was nicht als Argument in die Funktion kam oder eine Konstante ist. In `multi_werte()` betrifft das `kategorie`. Das kommt auf magische Weise aus dem „nichts”.

Ein Grund warum man auf Modulebene keinen Code haben sollte der nicht nur zum definieren von Konstanten, Funktionen, oder Klassen dient. Wenn der Hauptcode auch in einer Funktion stecken würde, könnte man auf `kategorie` nicht so zugreifen.

Backslashes ('\') zum fortführen von „logischen” Zeilen sind nur notwendig wenn es keine noch nicht wieder geschlossenen runden, eckigen, oder geschweiften Klammern gibt. Innerhalb des literalen Wörterbuchs für `filmdaten` erkennt der Compiler auch ohne den '\', dass die logische Zeile noch nicht zu ende sein kann.

Bei Funktionen hat es sich bewährt Namen zu wählen die Tätigkeiten beschreiben, denn genau das setzt man mit ihnen um. Also die erste Funktion müsste demnach `abfragen()` heissen. So kann man später Funktionen (und Methoden) besser von Objekten die zum Beispiel Dinge beschreiben, wie Klassen oder Exemplare, schon am Namen unterscheiden.

Die ersten beiden ``if``/``elif``-Zweige im Hauptcode haben den selben Code als Inhalt. Solche Wiederholungen sollte man vermeiden und beide Bedingungen zusammen fassen.

Die Einrückung sollte vier Leerzeichen pro Ebene betragen. Das wird in PEP 8 -- Style Guide for Python Code empfohlen und da Einrückung bei Python wichtiger Bestandteil der Syntax ist, sollten das alle gleich machen, damit man problemloser zusammen arbeiten kann.

[gofid]

Das ist ja schon mal einiges . Danke Dir. Werd das gleich mal anpassen.

[gofid]

Hallo nochmal,

@blackjack
also nochmal Danke für deine Tipps das hat mir wirklich sehr geholfen. Ich habe daraufhin meinen Code nochmal komplett überarbeitet und (hoffentlich) passendere Namen für die Funktionen und Variablen gefunden. Ausserdem hab ich, wie von Dir vorgeschlagen, die Funktionen etwas verallgemeinert.

Jetzt hab ich allerdings ein Problem und ich hab keine Idee warum das so ist. In der "Interaktiven Schleifensteuerung" wird nur die "else" Anweisung ausgeführt. Dabei seh ich von der Logik her keinen grossen Unterschied zu meiner vorherigen Version. Noch dazu gibt mir ein "print(eingabe)", wenn ich ihn in die Schleife einbaue, den richtigen String ( also hier ein "n" oder "j" ) aus. Und ein "type(eingabe)" bestätigt mir ebenfalls das es ein String ist.

Und ist es eigentlich geschickter in der "Interaktiven Schleifensteuerung" bei der "else"-Anweisung die Funktion noch mal aufzurufen oder sollte ich dort lieber sowas wie "eingabe = input(input('\nGeben Sie j oder n an \n')" eingeben?

Code: Alles auswählen

'''Interaktive Schleifensteuerung'''
def schleife_steuern(eingabe):
	while True:
		if eingabe == 'n' or eingabe == 'N':
			return False
		elif eingabe == 'j' or eingabe == 'J':
			return True
		else:
			schleife_steuern(input('\nGeben Sie j oder n an \n'))

'''Liste per Benutzereingabe mit x Werten füllen'''
def werte_speichern(wert_typ):
	werte = []
	while True:
		werte.append(input(wert_typ + ': '))
		if not schleife_steuern(input('\n' + wert_typ + \
		' hinzufügen? j/n \n')):
			return werte

'''Werte in Dictionary einfügen'''
def werte_abfragen(film_daten):

	'''Werte durch Benutzereingabe einfügen'''
	print('Geben Sie folgende Filmdaten ein:')

	for schluessel in film_daten.keys():

		if schluessel == 'Genre' or schluessel == 'Schauspieler':
			x_werte = werte_speichern(schluessel)
			film_daten[schluessel] = x_werte

		elif schluessel == 'Jahr':
			while True:
				try:
					film_daten[schluessel] = int(input(schluessel + \
					': '))
					break
				except:
					print('\nSie müssen eine Zahl eingeben\n')

		else:
			film_daten[schluessel] = input(schluessel + ': ')

	return film_daten

filminfos = {'Titel':'', 'Regiesseur':'', 'Jahr':'', 'Bewertung':'',
			'Genre':'', 'Schauspieler':''}

filme = werte_abfragen(filminfos)
print(filme)

Gruss
Gofid

PS: War mir jetzt nicht sicher ob ich das hier noch in den "Code-Vorstellungs-Thread" gehört, aber wollte dafür auch keinen neuen Thread aufmachen. Also wenn das hier nun zu Fragelastig wird, wär es nett wenn einer der Mods den Thread verschieben könnte.

[Hyperion]

Zu Deiner Frage: Das liegt an der ungewollten Rekursion! Das ganze funktioniert, so weit man immer sofort korrekt "j" oder "n" angibt. Wenn nicht, wird die Funktion ja rekursiv aufgerufen!

Ergänze Deinen Code mal um die letzte print-Funktion.

Code: Alles auswählen

def schleife_steuern(eingabe):
        while True:
                if eingabe == 'n' or eingabe == 'N':
                        return False
                elif eingabe == 'j' or eingabe == 'J':
                        return True
                else:
                        schleife_steuern(input('\nGeben Sie j oder n an\n'))
                        print("Bin immer noch hier :-(")

Wenn Du nun einmal "Mist" antwortest und in die Rekursion gelangst, wirst Du sehen, dass das `print` abgearbeitet wird!

Diese Art Rekursion solltest Du vermeiden - u.a. auch, weil irgend wann der Stack voll ist und das ganze dann kracht

Du siehst aber alleine am Aufruf von außen, wie ungünstig das ganze ist: Du musst dort ja immer wieder den `input`-Kram in den Aufruf schreiben - das ist ja wenig komfortabel!

Binde doch einfach den Rückgabewert von `input` an einen Namen (z.B. Dein `eingabe`) packe das unmittelbar unter das `while`.

Zum Rest gäbe es noch viel zu sagen... das überlasse ich anderen

[gofid]

Code: Alles auswählen

'''Interaktive Schleifensteuerung'''
def schleife_steuern():
	while True:
		eingabe = input('Weiteren Wert hinzufügen?')
		if eingabe == 'n' or eingabe == 'N':
			return False
		elif eingabe == 'j' or eingabe == 'J':
			return True
		else:
			print('Geben Sie j oder n an.')

'''Liste per Benutzereingabe mit x Werten füllen'''
def werte_speichern(wert_typ):
	werte = []
	while True:
		werte.append(input(wert_typ + ': '))
		if not schleife_steuern():
			return werte

@Hyperion
Die Print Anweisung hat er bei mir nicht ausgeführt. Soweit kam er anscheinend nicht.
Hab die beiden Passagen nun wie von Dir vorgeschlagen umgeändert. Dennoch führt er nur die "Else"-Anweisung aus. Egal ob ich "Mist" eingebe oder nicht.

[Hyperion]

Zum Rest gäbe es noch viel zu sagen... das überlasse ich anderen

Ok, hab mich doch mal aufgerafft

Also zuerst:

Die Einrückung sollte vier Leerzeichen pro Ebene betragen. Das wird in PEP 8 -- Style Guide for Python Code empfohlen und da Einrückung bei Python wichtiger Bestandteil der Syntax ist, sollten das alle gleich machen, damit man problemloser zusammen arbeiten kann.

Das hast Du wohl übersehen? Bitte besser dran halten!

Generell: Du solltest Dir eine universelle (evtl. zu schwer für einen Anfänger) oder drei spezialisierte Eingabe-Funktionen bauen. Eine für Texteingaben, eine für Zahlenwerte und eine für Ja / Nein-Entscheidungen. Das könnte so aussehen:

Code: Alles auswählen

def get_strings(question="Your input:"):
    return input(question)

def get_int(question="Your value:"):
    while True:
        try:
            return int(input(question))
        except ValueError:
            print("Bitte nur Integerwerte eingeben!")

def get_yes_no(question="Your value:"):
    result = {True: "y", False: "n"}
    while True:
        answer = input(question).lower()
        if answer in "yn":
            return result[answer]
        print("Bitte nur 'y' oder 'n' eingeben!")

(ungetestet) Damit hast Du nun drei Funktionen, die letztlich universell für alle Deine Eingaben benutzt werden können. (Diese sind auch nicht optimal, da man dort noch Code-Wiederholungen hat; das kann man noch besser zusammenfassen, aber dann wirds ein wenig komplizierter)

Deine Namen sind teilweise noch schlecht. `werte_speichern` sagt nicht viel aus! Du willst Du mehrere Werte für eine Kategorie abfragen. Auf Englisch würde ich so etwas wie `get_multiple_values` oder `get_multiple_tags` benutzen.

Du legst Dein Dictionary "falsch" an. Es gibt `dict.fromkeys`:

Code: Alles auswählen

In [50]: filminfo = dict.fromkeys(['Titel', 'Regiesseur', 'Jahr', 'Bewertung', 'Schauspieler', 'Genre'])

In [51]: filminfo
Out[51]: 
{'Bewertung': None,
 'Genre': None,
 'Jahr': None,
 'Regiesseur': None,
 'Schauspieler': None,
 'Titel': None}

Da Du die Einträge mit unterschiedlichen Datentypen belegst, ist diese Variante semantisch sinnvoller. Denn bei "Jahr" hast Du ja gar keinen String per default, sondern speicherst Integer Werte. Zudem kommen ja noch Listen bei `Genre` und `Schauspieler` hinzu.
Da Du die Schlüssel fest an die Funktionalität der Funktion `` koppelst, würde ich das Dictionary `filminfos` auch erst in der Funktion erstellen. Vorteil ist zudem, dass Du dann bei einem erneuten Aufruf ein neues Exemplar zurückbekommen wirst. Außerdem kann man das Dictionary dann auch gleich `film` nennen

In der vorletzten Zeile solltest Du besser `film` verwenden, da es sich ja nur um einen Film handelt

String-Konkatenation via `+` Operator ist meist suboptimal. Nutze doch die `format`-Methode:

Code: Alles auswählen

# nicht so gut
input(schluessel + ': ')
# besser
input('{}:'.format(schluessel))

Doc-Strings gehören unter einen Funktionskopf, nicht drüber!

Vermeide unnötige Namen:

Code: Alles auswählen

# das ist unständlich
x_werte = werte_speichern(schluessel)
film_daten[schluessel] = x_werte
# besser:
film_daten[schluessel] = werte_speichern(schluessel)

Sollten es zu viele Aufrufe sein, oder man will etwas leserlicher demonstrieren, dann sollte man natürlich "Zwischennamen" benutzen. Aber wenn das dermaßen unmittelbar aufeinander folgt, kann man sich das auch sparen.

Verwende niemals `except` ohne eine spezielle Exception anzugeben! Du "verschluckst" dann einfach alle Ausnahmen, was Ärger beim Debuggen machen kann (denn Ausnahmen, die Du nicht erwartest, willst / musst Du ja später kennen / sehen, um Deinen Code zu verbessern).

Ansonsten noch der Hinweis, dass man Code auf Moduleben so verbannen kann:

Code: Alles auswählen

def main():
    # hier kommt Dein "Einstiegs"-code rein

if __name__ == "__main__":
    main()

Zudem gehört es sich bei vollständigen Programmen, auch den She-bang und die Kodierung im Kopf anzugeben:

Code: Alles auswählen

#!/usr/bin/env python
# coding: utf-8

Oder bei Python3:

Code: Alles auswählen

#!/usr/bin/env python3
# coding: utf-8

[Hyperion]

@Hyperion
Die Print Anweisung hat er bei mir nicht ausgeführt. Soweit kam er anscheinend nicht.

Kann nicht sein! Dann hast Du die Zeile falsch eingerückt... sowohl in der `else`- als auch in der `while`-Ebene sollte das ausgegeben werden.

Hab die beiden Passagen nun wie von Dir vorgeschlagen umgeändert. Dennoch führt er nur die "Else"-Anweisung aus. Egal ob ich "Mist" eingebe oder nicht.

Kann ich ebenfalls nicht bestätigen. Bei mir funzt das jetzt genau so wie erwartet!

[gofid]

Gott, ich bin so schlecht.
ich dachte das bisschen Code wäre jetzt ganz ok... das wird ein sehr langer Weg für mich.

Danke Dir für deine genauen Ausführungen. Werd zusehen das ich mich daran halte.
Die Tabs habe ich in meinem Editor auf 4 Zeichen eingestellt, aber scheinbar is das beim kopieren verloren gegangen.

Kann nicht sein! Dann hast Du die Zeile falsch eingerückt... sowohl in der `else`- als auch in der `while`-Ebene sollte das ausgegeben werden.
Kann ich ebenfalls nicht bestätigen. Bei mir funzt das jetzt genau so wie erwartet!

Eben, ich versteh's auch nicht. Schließlich hat der gleiche Code vorher funktioniert. Hab das Skript nun auch mal auf einen anderen Rechner kopiert und dort ausgeführt und da hat's dann auch geklappt. :K

[Hyperion]

Gott, ich bin so schlecht.
ich dachte das bisschen Code wäre jetzt ganz ok... das wird ein sehr langer Weg für mich.

Unsinn! Jeder hat mal angefangen; das ist absolut normal, dass man am Anfang vieles suboptimal oder gar falsch gestaltet! Nicht verzagen, denn durch diese Erfahrungen lernst Du dazu.

Die Tabs habe ich in meinem Editor auf 4 Zeichen eingestellt, aber scheinbar is das beim kopieren verloren gegangen.

Du musst Deinem Editor sagen, dass er anstelle von einem Tab 4 Spaces einfügen soll! Vermutlich hat der Editor jetzt nur die Darstellung eines Tabs auf 4 Zeichen eingestellt.

Eben, ich versteh's auch nicht. Schließlich hat der gleiche Code vorher funktioniert. Hab das Skript nun auch mal auf einen anderen Rechner kopiert und dort ausgeführt und da hat's dann auch geklappt. :K

Nur damit wir uns nicht missverstehen: Das erste Beispiel sollte Deinen Fehler demonstrieren. Das zweite läuft prinzipiell richtig. Evtl. bist Du mit dem Kopieren durcheinander gekommen oder hast die falsche Datei gestartet?

[gofid]

Du musst Deinem Editor sagen, dass er anstelle von einem Tab 4 Spaces einfügen soll! Vermutlich hat der Editor jetzt nur die Darstellung eines Tabs auf 4 Zeichen eingestellt.

Das wird's vermutlich sein. Muss mal gucken wo ich das in dem Editor einstellen kann.

Nur damit wir uns nicht missverstehen: Das erste Beispiel sollte Deinen Fehler demonstrieren. Das zweite läuft prinzipiell richtig. Evtl. bist Du mit dem Kopieren durcheinander gekommen oder hast die falsche Datei gestartet?

Nein. Ich hab die gleiche Code-Datei (is auf einem USB Stick) auf zwei Rechnern ausprobiert und beim einen wurde immer nur die Else-Anweisung ausgeführt. Werd das morgen noch mal probieren, ob der Fehler reproduzierbar is. Heute komm ich an den Rechner nicht mehr ran. Allerdings hab ich beim einen Rechner den Editor (geany) zum ausführen benutzt, beim anderen Rechner hab ich die Datei einfach per Doppelklick ausgeführt. Vielleicht war auch das dass Problem.

[gofid]

Morgen,

hab's grad noch mal getestet und es ist tatsächlich so. Das Skript funktioniert auf meinem Windows 7 Rechner und auf meinem XP Rechner bleibt er in der "Else"-Anweisung hängen. Hab den XP Rechner mal gebootet, aber das hat auch nichts gebracht. Da kann man natürlich lang am Code rumfummeln, wenn die Basis schon nicht funktioniert. Sehr kurios das Ganze.

[Hyperion]

Starte das ganze bitte mal aus einer Shell und prüfe zuvor dort die Python Version mittels:

Code: Alles auswählen

python -V

Evtl. verwendest Du da unterschiedliche? Ich könnte mir denken, dass es bei Python 2.x wegen des `input` zu einem unerwarteten Ergebnis kommt. Sollte dem so sein, kannst Du mal mit einem

Code: Alles auswählen

print repr(eingabe)

angucken, was da wirklich drin steht!

Also evtl. ist auf dem einen Rechner Python 2.x installiert. Das wäre für mich jetzt der erste Gedanke.

[gofid]

OK, der print(repr(eingabe)) hat Licht ins Dunkel gebracht. Da gibt er mir ein 'j\r' zurück. Wie kann ich verhindern das er den Return in der Variable speichert? Das macht er allerdings nur wenn ich das in den command prompt von Windows eingebe. Mit IDLE ausgeführt, funktioniert der Code.

[Hyperion]

Was sagt denn die Version? Ist es nun Python 3.x beide Male oder einmal ein 2.x?

[deets]

Das kannst du nicht verhindern, du musst dann den String entsprechend praeparieren. Mit "'foo\n'.strip()" sollte das gehen.

[Hyperion]

Das kannst du nicht verhindern, du musst dann den String entsprechend praeparieren. Mit "'foo\n'.strip()" sollte das gehen.

Laut Doku sollte das aber nicht passieren: Link. Oder entfernt Python sogar das "\n" und übersieht nur das "\r", welches iirc unter Windows zusätzlich das Zeilenende markiert? Wenn ja würde ich das ja für einen Bug bzw. ein nicht durchdachtes Verhalten halten.

[gofid]

[...]Wenn ja würde ich das ja für einen Bug bzw. ein nicht durchdachtes Verhalten halten.

Na toll, da stosse ich gleich am Anfang auf solche Probleme... ich Glückspilz Danke dir nochmal, ohne deinen Tipp mit dem repr() hätte ich da vemutlich noch einige Stunden investiert...

Ist übrigens beide male Version 3.2 bei meinen Rechnern. Beim Windows 7 Rechner ist allerdings eine 64bit Version installiert.

@deets
Danke für den Tipp mit dem strip(), das hat geklappt. Ich muss mir mal eine Liste mit den ganzen BIF's auf meinen Arm tätowieren. Denn eigentlich kannte ich strip() ja schon.

[smodo]

Ich hab mir natürlich zum üben auch eine kleine datenbank erstell, es funktioniert auch alles, eig.
Es werden alle Film-Titel in einer .txt gespeichert. nur die funktion um film-titel zu löschen geht nicht.

Die sieht folgendermasen aus:

http://paste.pocoo.org/show/523917/

Ich kann filme suchen, filme hinzufügen, alle filme anzeigen lassen, nur löschen kann ich keinen. Es kommt immer "Film nicht vorhanden"... Liegt der fehler in der funktion ?

[BlackJack]

@smodo: Ja der Fehler liegt in der Funktion. Das geht so sowieso so nicht. Dateien haben keine `remove()`-Methode — man kann aus Textdateien nicht einfach mitten drin eine Zeile löschen.