BlackJack hat geschrieben:@snafu: Der Quelltext ist ja im Debian-Paket. Da ist auch Python dabei. Ich habe mal reingeschaut -- und mir sofort gewünscht ich hätte es nicht getan.
Wie war das unlängst mit dem "Arschigkeitsfaktor"
?
Im Gegnsatz zu euch war ich auch so mutig und habe es mal gestartet
Daher mal ein paar konstruktive Sachen (@solar22):
Erst mal Daumen hoch, dass Du ein Projekt auf die Beine stellst. Auch wenn es da noch viel Verbesserungspotenzial gibt; inhaltliches dabei mal ausgeklammert, da ich zum einen kein GeoCacher bin und zum anderen ich keine Caches im Programm angezeigt bekomme.
Als erstes fällt auf, dass die GUI ziemlich breit beim Start daher kommt. Dürften so knappe 1280 Pixel sein; ok mein Monitor ist auch noch ein alter 19"-4:3-Modell. Aber Du nutzt keinen Layoutmanager - wieso nicht? Speziell mit dem QtDesigner ist das doch ein Kinderspiel, ein schickes Layout zu basteln.
Du benutzt ein QMainWindow aber nicht dessen Funktionen! Wieso nutzt Du die Toolleiste (heißt das so?) nicht? Damit erspart man sich diese unschönen Buttons! Ich habe mal ein wenig gebastelt und am Layout geschraubt. Einiges würde ich generell anders lösen (z.B. den Webview mit dem sonderbaren Schließen-Knopf).
Link
Auf technischer Seite fiel mir sofort auf, dass Du das itembasierte QTableWidget nutzt. Auch wenn's am Anfang hart ist, aber ich würde das weghauen und auf das modelbasierte QListView / QTableView setzen. (Imho willst Du ein ListView). So schwer ist es gar nicht, sein eigenes Model zu erstellen; zumal Du ja keine großen Sperenzien drin hast und daher einfach auf das QStandardItemModel zurückgreifen kannst.
Allerdings bietet Qt Dir bereits
spezielle Models für Datenbankmodelle an! Dazu verpackst Du das dann noch in ein
QSortFilterProxyModel und Du bist alle Sorgen des manuellen Filterns los. Mich hatte
lunar seinerzeit auf die richtige Linie gebracht. Das hat sich echt gelohnt damals. Ich finde den Code grad nicht mehr, aber bei Interesse suche ich den gerne noch mal raus.
(Ha, beim Suchen im Board gefunden:
Wurde Dir schon einmal empfohlen!)
Bei einer ComboBox gibt es doch auch die Möglichkeit, indexbasiert auszuwerten. So etwas geht imho gar nicht:
Code: Alles auswählen
def loadTableItems(self,auswahl):
if auswahl == "alle Caches":
sql = "SELECT * FROM caches"
... # gekürzt
if auswahl == "meine Caches":
sql = ("SELECT * FROM caches WHERE `owner` = '%s' OR `id` = 'DUMMY'" % self.config[1][1])
if auswahl == "Favoriten":
sql = "SELECT * FROM caches WHERE `favo` = '1'"
if auswahl == "Suche":
sql = ("SELECT * FROM caches WHERE name LIKE '%"+str(self.searchQuery.text())+"%' OR wp LIKE '%"+str(self.searchQuery.text())+"%' OR owner LIKE '%"+str(self.searchQuery.text())+"%'")
Ich würde mir die Kommandos wohl in einer Liste merken und dann per Index das richtige auswählen. Da einige dynamisch sind, muss man ggf. auch eine lambda- oder eine echte Funktion in der Liste speichern. (Da Du aber ja hoffentlich auf das SQL- und das ProxyFilter-Model umsteigen wirst, erübrigt sich das ja dann eh
)
Die String-Konkatenation ist übrigens inkonsistent. Mal mittels "%", mal per "+". Beides würde ich zugunsten von "".format() verwerfen. Davon abgesehen sollte man SQL-Queries nicht manuell zusammenbasteln, sondern die dafür vorgesehenen Techniken verwenden. Stichwort SQL-Injection. (Ok, bei der lokalen App nicht so dramatisch, aber fürs "Leben" ist es doch besser, es immer und überall sauber zu implementieren):
Code: Alles auswählen
# Im Falle von SQLite ist das iirc so korrekt
sql = r"SELECT * FROM caches WHERE owner=? OR id='DUMMY'"
# später dann
cur.execute(sql, (self.config[1][1],))
# man beachte das *Tupel* als zweiten Parameter, speziell wenn man
# nur einen Parameter ersetzen will ist das eine böse Falle
Je nach DB variiert die genaue Syntax. Näheres dazu findest Du in der Doku.
Generell mag ich übrigens keine "*"-SELECTs. Ich würde da explizit immer alle Attributsnamen hinschreiben.
Das hier sieht auch strange aus:
Code: Alles auswählen
def loadConfig(self):
self.cur.execute("SELECT * FROM config")
self.cur.fetchone()
self.config = {}
n = 1
for row in self.cur:
self.config[n] = row
n = n +1
Du baust Dir ein Dict, welches als Key einen Integerwert enthält, der aber ein aufsteigender Index ist. Dazu braucht es kein Dictionary! Das kannst Du auch einfach in einer Liste speichern. Der Zugriff bei der SQL-Query oben zeigt ja, wie umständlich das ist. Zumal Deine Struktur in der DB-Tablle "config" ja aus den Attributen "name" und "value" besteht
Was liegt da näher, als den "namen" als Key und den "value"-Eintrag als value-Wert des Dicts zu nehmen?
Ob man diese Config wirklich in der DB halten muss und dazu kein simples INI- oder JSON-File verwendet ist sicher Geschmackssache.
Dieses loadConfig() bzw. den Code daraus habe ich auch noch woanders im Code gesehen - vermeide solche Redundanzen.
Deine __init__()-Methode aus Cachebase sieht auch falsch eingerückt aus.
Ich würde wohl auch auf die neue connect-Syntax zurückgreifen und das mal umstellen. Speziell falls Du mal auf PySide migrieren willst bietet sich das an.
Generell ist das ganze Projekt ein ziemliches Datei-Durcheinander! Das solltest Du dringend mal aufräumen. Ordne die Dateien einfach mal sinnvoller, lösche nicht mehr benötigte Sachen. Source-Code würde ich in einen "/src"-Ordner packen, Beispiel-Dateien in "/examples" o.ä.
Als letztes: Wozu brauchst Du in Gootes Namen PHP?
Das musst Du uns hier wirklich mal erklären
So, nach dem langen (unvollständigen) Review brauche ich erst mal nen starken Tee
passiert.
