Projekt Zeitmanagement

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
BlackJack

@T.T. Kreischwurst: Bei den Tabellennamen würde ich die Einzahl wählen, denn letztendlich ist so eine Beschreibung wie bei Klassen die Beschreibung *eines* Eintrags. Spätestens wenn man ein ORM verwendet fällt einem diese Parallele sehr deutlich auf.

Bei den IDs bist Du bei den Namen inkonsistent was Ein- und Mehrzahl angeht. Ich würde Präfixe dort weglassen wenn sie den eigenen Tabellennamen wiederholen, denn dann hat der Präfix keinen Mehrwert. Also in beiden Tabellen einfach nur `id` für den Primärschlüssel.

Gross-/Kleinschreibung würde ich bei SQL-Datenbanken vermeiden. Manche unterscheiden das, manche nicht, bei manchen hängt es von der Konfiguration ab. Kleinbuchstaben mit Unterstrichen funktionieren bei allen gleich.

`times` würde ich `slot` oder `time_slot` nennen damit man nicht für Tabelle, also ein Objekt, den gleichen Namen hat wie für ein Attribut des Objekts. Das führt dann gerne mal zu Verwirrungen ob an den Namen `time` jetzt so ein komplettes Objekt oder nur der Wert des `time`-Attributs gebunden ist.

Generell ist es eine gute Idee die Namen zwischen Programm und Datenbank zu ”synchronisieren”. Wenn das in der Datenbank `job_title` heisst, sollte es im Programm nicht `job` oder `name` heissen, sondern auch `job_title`. Oder eben auch in der Datenbank `name` wenn man es im Programm `name` nennt. Oder nur `title`, denn `job.job_title` ist auch wieder redundant und enthält nicht mehr Informationen wie `job.title`. Bei Fremdschlüsseln macht so ein Präfix vor `_id` Sinn, damit man sieht in welche Tabelle der Schlüssel verweist. Also eher `slot.job_id` als `slot.job_done`. Von dem Namensmuster würde ich erst abweichen wenn es mehr als einen Fremdschlüssel in einer Tabelle auf eine andere gibt.

`time` sollte auch in der Datenbank den entsprechenden Typ haben, also TIMESTAMP oder DATETIME. Und man sollte `sqlite3` dann auch sagen das solche Typen zu Python's `datetime.datetime` abgebildet werden sollen. Ich vergesse immer wieder wie man das macht, weil SQLAlchemy schon automatisch das ”richtige” tut. :-)

Da die Jobtitel/-namen nicht eindeutig sind, geht das mit `map_job_to_id()` und `remove_job()` so nicht. Beim Abfragen nennst Du den Wert jobID obwohl a) bei der Abfrage mehr als eine ID herum kommen kann und b) der Rückgabewert von `cursor.execute()` nicht definiert ist. Du willst da vielleicht vom Cursor das Ergebnis abfragen und ihn auch wieder schliessen bevor die Methode verlassen wird.

Bei anderen Methoden machst Du diesen Fehler auch.

Du verwendest Name oder Zeit als Identifizierung von Datensätzen. Dafür sind aber die IDs da! `remove_job()` löscht *alle* Jobs mit dem gleichen Namen/Titel. Die DB-API 2.0 beschreibt als optionale Erweiterung ein `lastrowid` auf `Cursor`-Objekten das die ID des zuletzt mit `execute()` eingefügten Datensatzes von diesem `Cursor` enthält. `sqlite3` hat das implementiert. Das sollte der Rückgabewert sein wenn man einen Datensatz anlegt. Beziehungsweise sollte das Bestandteil des Rückgabewertes sein wenn man anfängt mit Klassen für die Werte im Programm zu arbeiten. Und mit der Zeit bastelt man sich so sein eigenes ORM. Erwähnte ich SQLAlchemy schon mal? ;-)

Bei `get_jobs()` fehlt der Tabellenname in der Abfrage.

Wenn das löschen der Zeiten nicht erwünscht ist wenn man einen kompletten Job löscht, dann sollte auch das löschen von `job`-Einträgen nicht erwünscht sein. Denn die Zeiten verweisen dann auf einen nicht mehr existierenden Eintrag in einer anderen Tabelle. Das löst man üblicherweise mit einem Flag das `job`-Einträge als ”gelöscht” oder nicht mehr ”aktiv” markiert. Eine weitere Möglichkeit wäre ein Datumsfeld welches den Zeitpunkt der Deaktivierung oder NULL enthält wenn das für die Verarbeitung der Daten wichtig ist. Dann kann man sich auch rückwirkend für beliebige Zeitpunkte die aktiven Jobs filtern. Falls Jobs auch ein Anfangsdatum haben, könnte man das natürlich auch vermerken.

Apropos Flags: Hier hat SQLAlchemy den Vorteil einen BOOLEAN-Typ zu unterstützen auch wenn die dahinterliegende Datenbank den Typ nicht kennt.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Ne Menge Holz...
@BlackJack: ich verstehe dein Feedback so, dass du im ersten Teil eher Design-/Stilfehler ansprichst und danach wirkliche Bugs, also semantische oder Syntaxfehler. Auf erstgenannte gehe ich nicht weiter ein, sondern versuche sie einfach umzusetzen.
Gross-/Kleinschreibung würde ich bei SQL-Datenbanken vermeiden. Manche unterscheiden das, manche nicht, bei manchen hängt es von der Konfiguration ab. Kleinbuchstaben mit Unterstrichen funktionieren bei allen gleich.
Frage: meinst du die Bezeichner der Tabellen und Spalten oder die eigentlichen SQL Statements (z.B. SELECT FROM WHERE usw.)? Die Großbuchstaben bei der SQL Syntax finde ich nämlich z.B. sehr angenehm, weil man so eigenes von SQL-spezifischem leichter/schneller trennen kann.
`time` sollte auch in der Datenbank den entsprechenden Typ haben, also TIMESTAMP oder DATETIME. Und man sollte `sqlite3` dann auch sagen das solche Typen zu Python's `datetime.datetime` abgebildet werden sollen. Ich vergesse immer wieder wie man das macht
http://pythoncentral.io/advanced-sqlite ... in-python/ Anleitung, wie's gemacht wird. Das erfordert demnach sowieso einige Anpassungen am Design, weil der Colname angepasst werden muss.
Da die Jobtitel/-namen nicht eindeutig sind, geht das mit `map_job_to_id()` und `remove_job()` so nicht. Beim Abfragen nennst Du den Wert jobID obwohl a) bei der Abfrage mehr als eine ID herum kommen kann und b) der Rückgabewert von `cursor.execute()` nicht definiert ist. Du willst da vielleicht vom Cursor das Ergebnis abfragen und ihn auch wieder schliessen bevor die Methode verlassen wird.
Hier bin ich mir nicht ganz sicher, ob ich dich richtig verstanden habe. Dass mehr als eine ID ausgegeben werden kann, habe ich gerafft: z.B. wenn der User einen Job mit gleichem Namen zweimal anlegt. Meinst du aber mit dem zweiten Teil, dass ich das fetchone()/fetchall() vergessen habe? Das stimmt in der Tat... :oops:
Du verwendest Name oder Zeit als Identifizierung von Datensätzen. Dafür sind aber die IDs da! `remove_job()` löscht *alle* Jobs mit dem gleichen Namen/Titel. Die DB-API 2.0 beschreibt als optionale Erweiterung ein `lastrowid` auf `Cursor`-Objekten das die ID des zuletzt mit `execute()` eingefügten Datensatzes von diesem `Cursor` enthält. `sqlite3` hat das implementiert. Das sollte der Rückgabewert sein wenn man einen Datensatz anlegt. Beziehungsweise sollte das Bestandteil des Rückgabewertes sein wenn man anfängt mit Klassen für die Werte im Programm zu arbeiten. Und mit der Zeit bastelt man sich so sein eigenes ORM.
Das verstehe ich auch nicht ganz, sorry... Das Programm soll ja alle Namen mit gleichem Titel löschen, denn identische Jobs können nur durch Nutzerfehler entstehen. Ansonsten soll jeder Job nur einmal vorhanden sein. Bzgl. des Rückgabewertes beim Anlegen eines Datensatzes: :arrow: :K
Wenn das löschen der Zeiten nicht erwünscht ist wenn man einen kompletten Job löscht, dann sollte auch das löschen von `job`-Einträgen nicht erwünscht sein. Denn die Zeiten verweisen dann auf einen nicht mehr existierenden Eintrag in einer anderen Tabelle. Das löst man üblicherweise mit einem Flag das `job`-Einträge als ”gelöscht” oder nicht mehr ”aktiv” markiert.
Das klingt interessant. Ich würde das so lösen, dass in remove_jobs() erstmal geprüft wird, ob es in "times" (werde ich noch umbennen) Einträge gibt, die diese JobID haben. Wenn ja, würde ich eine neue Methode aufrufen, die die Flags entsprechend setzt (weiß allerdings nicht, wie genau das aussehen soll!) und dann den Job aus der Tabelle streicht. Wenn nicht, wird der Job "normal" gelöscht.
BlackJack

@T.T. Kreischwurst: Bezeichner sind Namen die der Benutzer frei vergeben kann um ”Dinge” zu bennenen. SELECT, FROM, WHERE, und so weiter sind Schlüsselworte. Und die schränken bei vielen Sprachen die Bezeichner ein, weil sie nicht als solche verwendet werden dürfen. Wenn man das nicht macht, dann kann man so schöne Sachen wie ``if if then else else then`` schreiben und der Benutzer darf dann herausfinden was Schlüsselwort ist und was eine Variable oder ähnliches bezeichnet. :-)

Ah, genau `PARSE_DECLTYPES` & Co war das. Wie gesagt, ich verwende SQLAlchemy, da braucht man sich um so etwas nicht kümmern. Da ”deklariert” man eine Spalte als DATE oder BOOLEAN und bekommt bei jedem DBMS `datetime.date`- und `bool`-Werte, egal ob die Datenbank das tatsächlich als eigenen Datentyp unterstützt oder nicht.

Richtig im zweiten Teil meinte ich `fetch*()` und vor allem mindestens bei `fetchone()` auch das schliessen des Cursors, denn sonst kann einem da eine Ausnahme um die Ohren fliegen. Es gibt DBMS die es nicht mögen wenn man einen Cursor nicht schliesst, weil das Ressourcen belegt die sonst nicht freigegeben werden. Wenn die Datenbank nicht weiss wann man mit einem Cursor fertig ist, kann es auch sein das eine Transaktion nicht beendet wird bis die Sitzung geschlossen wird.

Wenn jeder Titel nur einmal vorkommen darf, dann sollte man gar nicht erst mehrfach den gleichen Speichern können. Da gehört dann ein UNIQUE-Constraint auf die Spalte mit dem Titel. Und auch dann würde ich im Programm mit der ID des Titels für Abfragen arbeiten und nicht mit der Zeichenkette. Und NOT NULL ist der dann auch.

Der Rückgabewert beim Anlegen eines Datensatzen sollte dessen ID sein. Denn das ist das was ihn eindeutig IDentifiziert und das in der Datenbank auch schnell und Effizient. Man kann auch mehr als die ID zurückgeben, zum Beispiel ein Objekt das die Werte des Datensatzen enthält. Und vielleicht auch eine Referenz auf das `db`-Objekt, denn dann kann man dem Objekt Methoden verpassen um sich selbst zu speichern oder um vom Objekt andere Objekte aus der Datenbank zu laden die mit diesem Objekt in Beziehung stehen. Wie gesagt, man bastelt sich so ziemlich schnell ein eigenes ORM. Was es mit SQLAlchemy schon fertig gibt. :-)

Welche Flag willst Du bei den `times` setzen? Sollen die einzeln (de)aktivierbar sein oder nur der Job mit allen seinen Zeiten? Im letzteren Fall braucht man so ein Flag nur beim Job. Denn ob eine Zeit aktiv ist oder nicht kann man ja dann ermitteln in dem man den zugehörigen Job danach fragt.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Ah, Antwort von Blackjack *grabs keyboard, opens editor* :D
Bevor ich eine angepasste Version des Codes poste: unklar sind mir aktuell eigtl. nur die Punkte "ID als Rückgabewert" und Flags.
Ich verstehe nicht ganz, was ich mit der zurückgegebenen ID nach dem Einfügen eines Datensatzes machen soll. Irgendwie validieren? :K Letztlich brauche ich die ID zwar, aber nicht unmittelbar nach dem Eintragen eines Datensatzes, sondern viel später. Z.B. wenn ich irgendwann ein Datum mit den zugehörigen Jobs anfragen will...
Bei den Flags würde ich mir das so vorstellen, dass ich den Job mit allen zugehörigen Zeiten deaktiviere, nicht einzeln. Dass also dann bei einer Zeit steht: "inaktiver Job" oder sowas in der Art. Die Flag wäre in meiner VOrstellung dann ein weiteres Feld in der Job-Tabelle, das standardmäßig den Wert 1 (aktiv) hat und bei Deaktivierung des Jobs auf 0 gesetzt wird. Beim Anfragen der Zeiten/Jobs muss dann zusätzlich der Wert der Flag geprüft und bei inaktiven Jobs die Ausgabe des "Jobtitels" entsprechend geändert werden. Keine Ahnung, ob das im Sinne des Erfinders ist - wäre mein Ansatz.
BlackJack

@T.T. Kreischwurst: Die ID identifiziert den Datensatz in der Datenbank. Also muss man sich die merken wenn man wieder etwas mit dem Datensatz machen möchte. Du scheinst an der Stelle immer wieder über den Jobtitel gehen zu wollen. Das geht hier in diesem speziellen Fall aber auch nur weil der Jobtitel ebenfalls ein Schlüsselkandidat ist. Das ist ja nicht immer der Fall, und muss auch nicht so bleiben, und ist auch nicht so effizient nachschlagbar wie die ID.

Flags: In Deinem vorletzen Beitrag klang das noch so als wenn Du den Job-Datensatz tatsächlich löschen wolltest und bei den Zeiten zu dem Job ein Flag setzen wolltest.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Demnach müsste sowohl beim Anlegen eines Jobs als auch eines Zeiteintrags jeweils die ID zurückgegeben, an die GUI weitergereicht und dort vom System mit einer "Rücküberetzung" in Klartext verbunden werden, damit der Nutzer weiß, was er tut und das System trotzdem (unabhängig vom Nutzer) mit den IDs arbeiten kann. :?:
In Deinem vorletzen Beitrag klang das noch so als wenn Du den Job-Datensatz tatsächlich löschen wolltest und bei den Zeiten zu dem Job ein Flag setzen wolltest
Ja, das war ursprünglich auch der Plan. Da ich aber keine Ahnung hab, wie ich mir das mit den Flags vorstellen soll, habe ich das so neu interpretiert. Bin da ehrlich gesagt verwirrt...
BlackJack

@T.T. Kreischwurst: Wenn man nur die ID zurück gibt, dann ist ja in aller Regel zwangsläufig der nächste Schritt, dass über die ID der Jobtitel abgefragt wird, damit man den anzeigen kann. Und dazu dann den Status falls man inaktive Jobs gar nicht oder anders darstellen möchte. Also könnte man beim Einfügen gleich alles zurückgeben. Beziehungsweise *zum* einfügen alles ausser der ID, die dann von der Einfügemethode gesetzt wird. Ich sehe das aber auch alles aus einer Sicht das man das gar nicht selber schreiben möchte. Ich würde ein `Job`-Objekt erstellen und das dann speichern (lassen).

Ich denke da in solchem Code (unvollständig und ungetestet):

Code: Alles auswählen

# ...

class Job(Base):
    __tablename__ = 'job'

    id = Column(INT, primary_key=True)
    title = Column(TEXT, nullable=False, unique=True)
    is_active = Column(BOOLEAN, nullable=False, default=True)


class TimeSlot(Base):
    __tablename__ = 'time_slot'

    id = Column(INT, primary_key=True)
    job_id = Column(ForeignKey(Job.id), nullable=False)
    start_time = Column(DATETIME, nullable=False)

    job = relationship(Job)

    @property
    def is_active(self):
        return self.job.is_active

# ...

    job = Job(title='Nase bohren')
    session.save(job)
    slot = TimeSlot(job=job, start_time=datetime.datetime(2016, 12, 14, 12, 15))
    session.save(job)
    session.commit()

    all_jobs = session.Query(Job).all()
    active_jobs = session.Query(Job).filter_by(is_active=True).all()

    today = datetime.date.today()
    end_of_today = today + relativedelta(days=1, seconds=-1)
    todays_slots = session.Query(TimeSlot).filter(
        TimeSlot.start_time.between(today, end_of_today)
    ).all()
Ohne ORM würde ich mich trotzdem grob an dieser Struktur orientieren.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Servus!

Nach einer dreimonatigen Pause, bedingt durch einen spontanen und sehr erfreulichen Jobwechsel (habe es als Quereinsteiger in die IT-Brachne, genauer gesagt in ein Ausbildungsprogramm im Bereich Mainframe geschafft) habe ich nun mein altes Pythonprojekt wieder ausgegraben.
Letzter Stand war ein Neuentwurf des Datenbankdesign bzw. der entsprechenden Klasse, und hier sind beim Schreiben Fragen aufgetaucht, schon bevor alle Methoden formuliert waren. Hier der aktuelle Stand:

Code: Alles auswählen

import sqlite3

class Database():
	
	def __init__(self, filepath)
		self.connection = sqlite3.connect(filepath)
		self.create_tables()
		
	def create_tables(self):
		cursor = self.connection.cursor()
		cursor.execute(''' 
			create table if not exists
			job
				(
				 id integer not null
				,bezeichnung text
				)
			;
			create table if not exists
			time_slot
				(
				  ref_job integer not null
				 ,von timestamp
				 ,bis timestamp
				)
			;''')
		cursor.execute('''
			alter table job
				add primary key (id)
			;
			alter table time_slot
				add foreign key zeit_job (ref_job)
				references (job)
				on delete set null
			alter table zeit
				add primary key (ref_job)
			;	
			''')
		self.connection.commit()
	
	def insert_job(self, title):
		cursor = self.connection.cursor()
		cursor.execute('''
			insert into
			job
			values
			(?)
			''', (title, )
			)
		self.connection.commit()
	
	def insert_time(self, begin, end, ref_job)
		cursor = self.connection.cursor()
		cursor.execute('''
		insert into
		time_slot
		values
		(?,?,?)
		''', (ref_job, begin, end)
		)
		self.connection.commit()
		
	def read_data(self, XXXXXX)	
- Das SQL lehnt sich etwas an DB2 an, da ich das in der Ausbildung gelernt habe. Funktioniert das so oder sind da Konstrukte drin, die SQLite nicht kennt? Habe das unterwegs ohne SQLite Referenz geschrieben... bin daher nicht sicher, ob so Sachen wie das nachträgliche Einsetzen der Foreign und Primary Keys so klappt. Uns wurde jedenfalls nahegelegt, erstmal die Tabellenstruktur zu erstellen und dann erst Schlüssel und Constraints anzulegen.

- Beim Lesen der DB gibt es mit diesem Design (das Sirius' Vorschlag folgt) ein Problem: Im Programm wird der Nutzer hauptsächlich über Datumsangaben zugreifen. Sprich: er wird sagen: "gib mir alle Jobs, die ich am 20.03.2017 erledigt habe". Das geht hier - aber nur, wenn man einen Vergleich auf ein nicht-Schlüsselfeld macht. Das soll man nicht tun, aber Datums-/Zeitangaben finde ich als Primärschlüssel schwierig. Man müsste dann z.B. das Feld "von" nehmen, im WHERE Feld des SQL Statements das Datum aus dem Schlüssel ziehen und mit dem übergebenen Datum vergleichen. Datetime Objekte kann ich nicht vom Programm an die Datenbank übergeben, weil sie zu kleinteilig sind: den Nutzer interessieren nur ganze Tage, nicht ein einzelner Zeitabschnitt davon.
Irgendwie ist das aber ungeil...
BlackJack

@T.T. Kreischwurst: Was ist denn die Begründung dafür Primär- und Fremdschlüssel erst in einem zweiten Gang bekannt zu machen? Da hat man dann ja nicht *eine* Beschreibung davon wie die Tabellen aussehen, sondern muss die sich aus den beiden Anweisungen erst zusammen suchen. Bei zwei Tabellen geht das vielleicht noch, aber bei einer DB mit vielen Tabellen fänd ich das sehr unübersichtlich. Wo ist die Deklaration der Tabelle `zeit`?

Bei SQLite kann das tatsächlich ein Problem werden, weil man bei vorhandenen Tabellen nicht alle möglichen Änderungen machen kann. (Ich weiss jetzt leider aus dem Kopf nicht welche Änderungen möglich sind und welche nicht.)

Warum soll man keine Abfragen auf Nicht-Schlüssel-Felder machen? Wenn das doch aber die Kriterien sind die den Benutzer interessieren? Komische Regel(‽)

Das Datum/die Zeitstempel würde ich nicht als Schlüssel verwenden, das macht speziell beim Datum auch gar keinen Sinn, denn es gibt ja mehr als einen Eintrag an einem Datum, also kann das Datum gar kein Primärschlüssel sein. Bei den zu erwartenden Datenmengen würde ich mir erst einmal gar keinen Kopf um so etwas machen und einfach nach dem `DATE()`-Teil vom Zeitstempel filtern. Sollte das dann irgendwann tatsächlich zu einem *messbaren* Problem werden, kann man einen Index auf die `von`-Spalte setzen und mit BETWEEN den ganzen Tag filtern. Aber das würde ich wirklich erst machen wenn es tatsächlich problematisch wird, was wohl ein paar Jahre an Datensammeln dauern kann.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Was ist denn die Begründung dafür Primär- und Fremdschlüssel erst in einem zweiten Gang bekannt zu machen?
Weiß ich nicht genau. Ich nehme an, es war ein Tipp für uns Anfänger: erstmal die Tabellen sauber erstellen, dann Contraints anlegen, dann nächste Tabelle. Bei sehr großen creates nehme ich an, dass auch der Ausbilder auf die Kurzschreibweise zurückgegriffen hätte. Er hat sie nämlich explizit erwähnt, allerdings stehen bei den besprochenen Änderungen an DB2 Systemen allenfalls einzelne Tabellen-Creates an, höchst selten mal mehr als zwei gleichzeitig. Die müssen aber passen.
Wo ist die Deklaration der Tabelle `zeit`?
Ups, das ist der alte Name. Habe sie in time_slots umbenannt, muss das hier noch ändern... :oops:
Dann werde ich die Schreibweise wieder auf die kürzere aus meiner vorherigen Version umstellen.
Warum soll man keine Abfragen auf Nicht-Schlüssel-Felder machen? Wenn das doch aber die Kriterien sind die den Benutzer interessieren? Komische Regel(‽)
Das ist keine Regel aus der Ausbildung, sondern eine Überlegung von mir. Es gab ja den Hinweis, am besten über IDs abzufragen. Ich sehe das wie du, obwohl das Datum prinzipiell schon keyfähig wäre. Es sind ja Datetime Objekte, und da nur ein Job zu einer Zeit gespeichert werden kann, sollte es gehen. Trotzdem lieber über Fremdschlüssel zu Job, das finde ich sauberer.

Ansonsten scheint es ja soweit zu passen - dann würde ich nämlich weiterschreiben.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

So, neue Version der Datenbank mit ein paar Fragen. Ich habe mit einer separaten Testklasse die Funktionen jeweils getestet und sie tun soweit, was sie sollen. Ich kann natürlich nicht garantieren, dass ich an alle möglichen Testfälle gedacht habe...
Aber zu den Fragen:
1) Ist es besser oder schlechter, erst den Primärschlüssel für den Job über read_job in einer eigenen Funktion abzufragen und dann mithilfe des jew. Rückgabewertes Aktionen wie ein delete (in remove_job) durchzuführen? Oder sollte ich mir die Funktion sparen und direkt in den Funktionen einen Stringvergleich machen? Also: in remove_job statt where id =? schreiben: where bezeichnung= ? und statt einer ID den vom Nutzer eingegebenen String prüfen? So wie ich es jetzt habe, ist es halt einheitliher, weil es genau eine Schnittstelle gibt, an der geprüft wird, ob es den vom Nutzer ausgewählten Job gibt. Allerdings ist es etwas uständlich und ich bin vom Nutzen noch nicht übrezeugt.
2) on delete Befehle funktionieren nicht beim Test. Das delete wird zwar durchgeführt, aber die Zeile in der Tabelle time_slot bleibt bei cascade einfach stehen und wird auch bei set null nicht null gesetzt.
Wo ist mein Denkfehler?

Code: Alles auswählen

import sqlite3

class Database():
	
	def __init__(self, filepath):
		self.connection = sqlite3.connect(filepath)
		self.create_tables()
		
	def create_tables(self):
		cursor = self.connection.cursor()
		cursor.execute(''' 
			create table if not exists
			job
				(
				 id integer primary key not null
				,bezeichnung text
				)
			;''')
		cursor.execute(''' 
			create table if not exists
			time_slot
				(
				  ref_job integer primary key not null
				 ,von timestamp
				 ,bis timestamp
				 
				 ,foreign key(ref_job)
				 references job(id)
				 on delete cascade
				)
			;''')
		self.connection.commit()
	
	def insert_job(self, title):
		cursor = self.connection.cursor()
		cursor.execute('''
			insert into
			job(bezeichnung)
			values
			(?)
			''', (title, )
			)
		self.connection.commit()
		
	def update_job(self, term, job_id):
		cursor = self.connection.cursor()
		cursor.execute('''
		update
			job
		set
			bezeichnung = ?
		where
			id = ?
		''', (term, job_id))
		self.connection.commit()
	
	def insert_time(self, begin, end, ref_job):
		cursor = self.connection.cursor()
		cursor.execute('''
		insert into
		time_slot
		values
		(?,?,?)
		''', (ref_job, begin, end)
		)
		self.connection.commit()
		
	def read_job(self, term):
		cursor = self.connection.cursor()
		job_id = cursor.execute('''
		select
			id
		from
			job
		where
			bezeichnung = ?
		''', (term,))
		job_id = job_id.fetchone()[0]
		cursor.close()
		return job_id
		
	def read_data(self, begin, end):
		cursor = self.connection.cursor()
		jobs_done = cursor.execute('''
		select 
			job.bezeichnung
		from
			job
			,time_slot
		where
			time_slot.ref_job = job.id
		and
			time_slot.von = ?
		and
			time_slot.bis = ?
		''', (begin, end))
		jobs_done = jobs_done.fetchall()
		cursor.close()
		return jobs_done
	
	def remove_job(self, job_id):
		cursor = self.connection.cursor()
		cursor.execute('''
		delete from
			job
		where
			id = ?
		''', (job_id,))
		self.connection.commit()
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

@T.T. Kreischwurst: ich weiß nicht, wie Du Dir Deine Oberfläche gestaltest, aber normalerweise fragt man ID und Bezeichnung ab, nimmt die Bezeichnung zur Darstellung aber die ID zum Arbeiten. Deshalb sollte insert_job auch die neu erzeugte ID zurückgeben. In insert_time würde ich die Spaltennamen noch explizit angeben. read_job hieße besser find_job_id und sollte noch prüfen, ob wirklich nur ein Job mit dieser Bezeichnung existiert. Soweit ich sehe ist die Spalte bezeichnung nicht unique. read_data ist ein zu generischer Name. Warum heißt das Ergebnis jobs_done? Auch hier solltest Du mit job_ids arbeiten, so dass der Join gar nicht nötig ist. Willst Du nicht nach Jobs suchen, für die gilt von<=datum<=ende? Normalerweise weiß man ja nicht, wann ein Job angefangen hat und wann er beendet wurde?

Ich designe Datenbanken normalerweise so, dass es kein remove gibt.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Deshalb sollte insert_job auch die neu erzeugte ID zurückgeben.
Erledigt. Stimmt, dann kann man die ID gleich benutzen, z.B. wenn man den Namen des neuen Jobs in der GUI anzeigen will.
Danke für den Hinweis:
read_job hieße besser find_job_id und sollte noch prüfen, ob wirklich nur ein Job mit dieser Bezeichnung existiert. Soweit ich sehe ist die Spalte bezeichnung nicht unique
Einen Job soll es nur einmal geben, daher bietet sich der unique constraint an. Ist eingebaut.
Warum heißt das Ergebnis jobs_done? Auch hier solltest Du mit job_ids arbeiten, so dass der Join gar nicht nötig ist. Willst Du nicht nach Jobs suchen, für die gilt von<=datum<=ende? Normalerweise weiß man ja nicht, wann ein Job angefangen hat und wann er beendet wurde?
Korrekt - so, wie sie jetzt da steht, soll die Funktion zu viel leisten, was sie gar nicht kann. Ich brauche in Bezug auf die Tabelle Job drei Funktionen/selects, die irgendwie Daten auslesen:
1) get_job_id: holt einen Datensatz für genau eine vom Nutzer eingegebene Bezeichnung, sofern vorhanden. Liefert sonst none zurück und kann daher als Prüfung dienen, ob es einen Job mit einer bestimmten Bezeichnung gibt oder nicht.

2) Eine Funktion, die alle Job-Bezeichnungen ausliest und zurückgibt: das ist für eine Auswahlliste in der GUI nötig, in der alle Jobs stehen.

3) EIne Funktion, die Jobs sucht, für die gilt von<=datum<=ende, wie du schreibst. Das ist das, woran ich bei read_data gedacht hatte.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

So. Ich habe die angemerkten Änderungen eingebaut und den Code in einem neuen Branch in Github eingestellt (Link im ersten Post).
Das ganze würde ich als eigenes Modul so stehen lassen; Frage ist, ob sich eine weitere Unterteilung in Klassen anbietet. Derzeit würde ich das nicht so sehen; einzig die Erzeugung der Tabellen könnte ich mir noch als eigene Klasse vorstellen, wobei man die beiden executes, welche die Tabellen erzeugen, dann zu eigenen Methoden machen würde. Wie gesagt, ich finde es aktuell nicht sinnvoll, außer die Database-Klasse ist zu lang.
Ansonsten ist mein Plan, jetzt die ganze Config-Dateien Logik und die Passwort Logik in eigenen Klassen zu schreiben und sauber von der GUI zu trennen. Die weiteren Schritte sind sehr eng mit der GUI verzahnt oder sind die GUI im engeren Sinn; das kommt also später.
__deets__
User
Beiträge: 14493
Registriert: Mittwoch 14. Oktober 2015, 14:29

Sieht schon ganz gut aus.

Wie immer milde Meckereien, bzw. Verbesserungsvorschlaege:

- deine Kommasetzung bei den Create-Statements ist ungewoehnlich und erschwert mir das Verstaendnis.
- ich bin ein Freund datengetriebener Programmierung. Statt in der Methode ewig lange SQL-statements zu haben, wuerde ich eher eine Liste auf Modulebene anlegen, "CREATE_STATEMENTS", und ueber die rueberlaufen, und die dann abfeuern.
- im Sinne von DRY faellt mir das oft am Ende einer Methode stehende commit auf. Das waere fuer mich ein klarer Kandidat fuer einen Dekorator-basierten Ansatz, bei dem ich damit klar kenntlich mache ob in einer Methode Daten veraendert werden, ohne das ich die Details kennen muss. Und umgekehrt: schreibe ich eine Methode, die Daten veraendern soll, ist es einfach, sie auszuzeichnen. Ungetestet so:

Code: Alles auswählen

from functools import wraps
def commits(func)
      @wraps(func)
      def _d(self, *a, **k):
           res = func(self, *a, **k)
           self.connection.commit()
           return res
       return _d
       
       
...

class Foo(...):


    @commits
    def neuer_job(...)
    
    
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

__deets__ hat geschrieben: Wie immer milde Meckereien, bzw. Verbesserungsvorschlaege:
Ich steh auf milde Meckereien. Deswegen bin ich hier :mrgreen: Danke fürs Feedback!
Die Kommasetzung ist zugegebenermaßen ungewöhnlich, hilft mir persönlich aber bei der Wartung des Codes. Setze ich die Kommata immer an die erste Position, stehen sie immer untereinander und immer am gleichen Fleck. So sehe ich sofort, wenn ein Komma fehlt oder überschüssig ist, wenn sie am SQL was geändert hat. Ich kann es künftig aber gerne auch klassisch schreiben, sofern ich dran denke :wink:
Statt in der Methode ewig lange SQL-statements zu haben, wuerde ich eher eine Liste auf Modulebene anlegen, "CREATE_STATEMENTS", und ueber die rueberlaufen, und die dann abfeuern
Du meinst also ein Modul bestehend lediglich aus einer Liste mit allen nötigen SQL Statements, die dann in den Python Funktionen nur mit Liste[Index] angesprochen werden? Klingt nice, aber wie ist das dann bei Parametern, die in das SQL reinmüssen? Deren Verwendung wird durch die Auslagerung in ein Modul ja eher erschwert, oder?
Deinen letzten Punkt blick ich ehrlich gesagt nicht :K
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

@T.T. Kreischwurst: Eingerückt wird mit 4 Leerzeichen pro Ebene, nicht mit Tabs. Jedesmal, wenn eine Instanz der Datenbankklasse erstellt wird, alles Tabellen zu erzeugen, halte ich für einen Fehler. Das sollte man bewußt, einmal beim erstellen einer neuen Datenbank machen. Zeile 51: wußte gar nicht, dass man nach einem INSERT ein fetch machen kann. Gerade ausprobiert, geht auch gar nicht. Das nimmt solangsam Ausmaße an, dass es ratsam wäre auf SQLAlchemy umzusteigen.
T.T. Kreischwurst
User
Beiträge: 52
Registriert: Dienstag 2. Februar 2016, 10:56

Ah ja da war etwas, das Blackjack mal erwähnt hatte.
Ich kann jetzt alles nochmal auf SQLAlchemy umschreiben, aber ist das wirklich sinnvoll? Erstens entstehen damit neue Fehlerquellen (weil unbekannt), zweitens dachte ich, ich lerne das ganze erstmal auf die rudimentäre, harte Tour, bevor ich Hilfsmittel benutze, mit denen ich bestimmte Fehler nicht mache. Ich bekomme daher eher immer stärker das Gefühl, dass die ganze Sache keinen Sinn hat. Ich komme hier zunehmend vom 100. ins 1000. und stelle fest, dass ich weder einen Plan von meinem Programm habe, noch einen erstellen kann weil ich keine Ahnung von objektorientiertem Entwurf habe und in letzter Konsequenz nicht begreife, was ORM eigtl. ist und was das für einen Vorteil bringen soll.
Vor diesem Hintergrund dann Prinzipien und Systeme anwenden und ausprobieren zu wollen, die man eigtl. nicht begreift, erscheint mir immer weniger sinnvoll. Wahrscheinlich muss ich erstmal viel mehr lernen, wobei ich nicht weiß, wo ich anfangen soll :K
Zuletzt geändert von T.T. Kreischwurst am Dienstag 18. April 2017, 17:09, insgesamt 1-mal geändert.
__deets__
User
Beiträge: 14493
Registriert: Mittwoch 14. Oktober 2015, 14:29

Bezueglich der statements: die sollen nicht (notwendigerweise) ausgelagert, aber getrennt vom Code werden;

Code: Alles auswählen


CREATE_TABLE_STATEMENTS = [
"""create table foo (
....
)""",
"""create table foo (
....
)""",

]

...

for statement in CREATE_TABLE_STATEMENTS:
     cursor.execute(statement)


Damit wird in meinen Augen der Code klarer - ich kann den losgeloest von deinen aufwaendig (was gut ist) formatierten SQL statements verstehen.

Das lohnt natuerlich nur bei statements die keine oder immer die gleichen Parameter haben, und wenn man eben wirklich mehrere hat - ich wuerde das nicht aus Prinzip empfehlen, sondern wirklich nur fuer die Anlage der DB.

AFAIK kann man sogar multi-statement absetzen, und statt einer Liste mehrere durch Semikola getrennte Statements in einen String schreiben.

Dann hat man einen Block, den man sogar per C&P von Hand in sqlite eingeben kann, wenn man das mal will.
Antworten