Hey,
für ein Projekt, habe ich einen NewsFeed geschrieben, der im Vollbildmodus gestartet wird, und dann die aktuellsten News herunterlädt und auf einem UI anzeigt, was in Tkinter geschrieben ist. Allgemine läuft alles so wie es soll und ich wollte einfach mal wissen ob und was ihr anders gemacht hättet, und wo ich einen schlechten Programmierstil pflege.
Hier habt ihr nen link zu ner zip datei in der alles drin ist!
https://dl.dropboxusercontent.com/s/w1q ... zQfVw&dl=1
Viel Spaß und danke
Kritik am code
Hey misterjosu,
warum erfindest Du ein seltsames neues News-Format? Ich weiß, die üblichen XML-basierten Formate sind für Deine Zwecke etwas überladen, aber das Format, das Du da erfunden hast ist einfach fehleranfällig, da »post_content« bestimmte magische Zeichen und Wörter nicht enthalten darf ( {, }, post_name=, usw. ). Warum nimmst Du nicht einfach JSON, dann brauchst Du auch nicht so einen kaputten Parser selbst schreiben.
Zur Hauptdatei: Sternchenimporte von »tk« wurden hier im Forum zurecht schon oft kritisiert. Label sollten auch nicht als Klassenattribute erzeugt werden und »mainloop« auf keinen Fall in »__init__« aufgerufen werden. Warum liest Du die news-Datei immer wieder neu?
warum erfindest Du ein seltsames neues News-Format? Ich weiß, die üblichen XML-basierten Formate sind für Deine Zwecke etwas überladen, aber das Format, das Du da erfunden hast ist einfach fehleranfällig, da »post_content« bestimmte magische Zeichen und Wörter nicht enthalten darf ( {, }, post_name=, usw. ). Warum nimmst Du nicht einfach JSON, dann brauchst Du auch nicht so einen kaputten Parser selbst schreiben.
Zur Hauptdatei: Sternchenimporte von »tk« wurden hier im Forum zurecht schon oft kritisiert. Label sollten auch nicht als Klassenattribute erzeugt werden und »mainloop« auf keinen Fall in »__init__« aufgerufen werden. Warum liest Du die news-Datei immer wieder neu?
@misterjosu: Was da alles beim erstellen des Klassenobjekts passiert und damit auch als Klassenattribute endet, gehört dort nicht hin.
Die Pfade sind nicht portabel, das läuft so wohl nur unter Windows.
Was mir der Kommentar ``# logical variables:`` sagen soll, verstehe ich nicht. Und danach kommt dann ``# mutable variables`` — aber Attribute die sehr danach aussehen als wenn sie *nicht* verändert werden‽ Der Kommentar ``# padding`` vor der Definition von `padding` ist sinnfrei. Da folgen dann noch zwei solcher Kommentare wo einfach nur noch mal der Name des Attributs davor noch einmal als Kommentar steht.
Der Inhalt von `FileHandle` ist — naja. Das wäre eine, höchstens zwei Funktionen gewesen. Der Sinn von `get_file()` erschliesst sich mir nicht. Und `phars_file()` scheint irgendwas selbstgebackenes zu parsen. (Sollte *das* vielleicht der Funktionsname sein: `parse_file()`?). Aber warum macht man so etwas, wo es doch ein `json`-Modul in der Standardbibliothek gibt? Bei den Fällen im Parser enthalten einige fast identischen Quelltext. Das verletzt das DRY-Prinzip (Don't Repeat Yourself).
Und es stellt sich auch die Frage warum kein Standardformat verwendet wird. Für News-Feeds gibt es RSS und ATOM. Und mit `feedparser` auch eine Bibliothek, die damit etwas anfangen kann.
Die Pfade sind nicht portabel, das läuft so wohl nur unter Windows.
Was mir der Kommentar ``# logical variables:`` sagen soll, verstehe ich nicht. Und danach kommt dann ``# mutable variables`` — aber Attribute die sehr danach aussehen als wenn sie *nicht* verändert werden‽ Der Kommentar ``# padding`` vor der Definition von `padding` ist sinnfrei. Da folgen dann noch zwei solcher Kommentare wo einfach nur noch mal der Name des Attributs davor noch einmal als Kommentar steht.
Der Inhalt von `FileHandle` ist — naja. Das wäre eine, höchstens zwei Funktionen gewesen. Der Sinn von `get_file()` erschliesst sich mir nicht. Und `phars_file()` scheint irgendwas selbstgebackenes zu parsen. (Sollte *das* vielleicht der Funktionsname sein: `parse_file()`?). Aber warum macht man so etwas, wo es doch ein `json`-Modul in der Standardbibliothek gibt? Bei den Fällen im Parser enthalten einige fast identischen Quelltext. Das verletzt das DRY-Prinzip (Don't Repeat Yourself).
Und es stellt sich auch die Frage warum kein Standardformat verwendet wird. Für News-Feeds gibt es RSS und ATOM. Und mit `feedparser` auch eine Bibliothek, die damit etwas anfangen kann.
-
- User
- Beiträge: 44
- Registriert: Samstag 29. Dezember 2012, 21:40
Ok. Also sollte ich dinge wie das erstellen der frames in die instruktor packen ? Das mit den standartformaten überleg ich mir, der parser ist auch noch nicht ganz so wie er sein soll, aber so hab ich das ganze eben wie ich es will. Die datei news.txt wird jedesmal neu eingelesen weil ich im modul keine globalen variablen haben wollte.
Der kommentar logic variables soll heißen dass diese variablen verwendet werden um den aktuell angezeigten post als zahl zu speichern. Mutable variables heißt einfavh dass man die url der news file usw. Hier ändern kann
Naja, jetzt weiß ich ja was ich morgen zutun hab.
Der kommentar logic variables soll heißen dass diese variablen verwendet werden um den aktuell angezeigten post als zahl zu speichern. Mutable variables heißt einfavh dass man die url der news file usw. Hier ändern kann
Naja, jetzt weiß ich ja was ich morgen zutun hab.
@misterjosu: Es gibt hier wohl probleme mit Begriffen. Instruktor ist wohl eine Mischung aus Init und Konstruktor? Den Begriff gibt es so nicht. Und statt zu erklären was *Du* mit logic variables und mutable meinst, solltest Du diese Begriffe vielleicht für etwas verwenden was sie tatsächlich bezeichnen. Denn zumindest unter mutable versteht man allgemein etwas anderes.
Was gibt es denn bei Standardformaten zu überlegen?
Was gibt es denn bei Standardformaten zu überlegen?
-
- User
- Beiträge: 44
- Registriert: Samstag 29. Dezember 2012, 21:40
so also ich hab das ganze mal überarbeitet, hier die NewsFeed.py:
Code: Alles auswählen
from tkinter import *
import FileHandler as FileHandler
import os
class Gui(object):
def __init__(self, root):
self.root = root
# set window to fullscreen
self.screenwidth = self.root.winfo_screenwidth()
self.screenheight = self.root.winfo_screenheight()
self.root.geometry("{0}x{1}+0+0".format(self.screenwidth, self.screenheight))
self.root.overrideredirect(True)
# backgroundcolor (is used in any widget)
self.color = "sea green"
# logical variables:
self.shown_post = 0
# variables (pleas change)
self.news_file = "news.txt"
self.url = "https://dl.dropboxusercontent.com/s/y6ys96go7jkj0zx/news_public.txt?token_hash=AAGaT5HXvVWvpN40skUMFn8dPdFE1yGCZfE9Ye6X6qykQw&dl=1"
# used images
self.header_image = PhotoImage(file="img\header.gif")
self.logo_image = PhotoImage(file="img\logo.gif")
self.close_image = PhotoImage(file="img\close.gif")
self.px = PhotoImage(file="img\pixel.gif")
self.l_arrow_image = PhotoImage(file="img\c_left_arrow.gif")
self.r_arrow_image = PhotoImage(file="img\c_right_arrow.gif")
self.uniwars_logo_image = PhotoImage(file="img\c_uniwars_logo.gif")
# padding of elements in Guis
self.padding = 2
# create frames
# --> l_sidebar_frame
self.l_sidebar_frame = Frame(self.root)
# --> post_frame
self.post_frame = Frame(self.root)
# create post structure
self.post_title_label = Label(self.post_frame, text="Title", bg=self.color, image=self.px, compound=LEFT, width=self.screenwidth-304, height=50, bd=1)
self.post_content_label =Label(self.post_frame, text="Text", bg="white", image=self.px, compound=LEFT, width=self.screenwidth-172-12, bd=1)
self.post_autor_label = Label(self.post_frame, text="Autor + Time", bg=self.color, image=self.px, compound=LEFT, width=self.screenwidth-172-12, bd=1)
# grid all frames
self.l_sidebar_frame.grid(row=1, column=0, sticky=NW)
self.post_frame.grid(row=1, column=1, columnspan=2, sticky=NW)
# fill header frame
# --> logo_label
self.logo_label = Label(self.root, image=self.logo_image, bg=self.color)
self.logo_label.grid(column=0, row=0, pady=self.padding, padx=self.padding)
# --> header_label
self.header_label = Label(self.root, image=self.header_image, bg=self.color)
self.header_label.grid(column=1, row=0, pady=self.padding, padx=self.padding, ipadx = (self.screenwidth-798-12)/2, sticky=NW)
# --> close_button
self.close_button = Button(self.root, image=self.close_image, bg=self.color, command=quit, relief="flat",bd=1)
self.close_button.grid(column=2, row=0, pady=self.padding, padx=self.padding, sticky=NW)
# create post
# --> l_arrow_bouuton
self.l_arrow_button = Button(self.post_frame, image=self.l_arrow_image, relief="flat", bd=1, bg=self.color, command=self.r_button)
self.l_arrow_button.grid(column=0, row=0, pady=self.padding, padx=self.padding)
self.r_arrow_button = Button(self.post_frame, image=self.r_arrow_image, relief="flat", bd=1, bg=self.color, command=self.l_button)
self.r_arrow_button.grid(column=2, row=0, pady=self.padding, padx=self.padding)
# --> grid post structure
self.post_title_label.grid(column=1, row=0, pady=self.padding, padx=self.padding)
self.post_content_label.grid(column=0, row=1, columnspan=3, pady=self.padding, padx=self.padding, ipady=20)
self.post_autor_label.grid(column=0, row=2, columnspan=3, pady=self.padding, padx=self.padding, ipady=10)
# fill labels with content(post)
FileHandler.download_file(self.url, self.news_file)
self.sync_posts(FileHandler.get_file(self.news_file))
# fillind l_sidebar with content
# --> download_label
dl_label = Label(self.l_sidebar_frame, text="-- Download's --", bg=self.color, image=self.px, compound=LEFT, width=168, height=50, bd=1)
dl_label.grid(column=0, row=0, pady=self.padding, padx=self.padding)
# --> l_sidebar (download links)
self.uniwars_dl_button = Button(self.l_sidebar_frame, image=self.uniwars_logo_image, relief="flat", bg=self.color, bd=1, command=self.open_sites)
self.uniwars_dl_button.grid(row=1, column=0, pady=self.padding, padx=self.padding)
mainloop()
def r_button(self):
if 0 < self.shown_post:
self.shown_post -= 1
self.sync_posts(FileHandler.get_file(self.news_file))
def l_button(self):
if len(FileHandler.get_file(self.news_file))-1 > self.shown_post:
self.shown_post += 1
self.sync_posts(FileHandler.get_file(self.news_file))
def sync_posts(self, shown_post):
posts = FileHandler.get_file(self.news_file)
self.post_title_label.config(text=posts[self.shown_post]["post_name"])
self.post_content_label.config(text=posts[self.shown_post]["post_content"])
self.post_autor_label.config(text="Geschrieben von: "+posts[self.shown_post]["post_autor"]+" am "+posts[self.shown_post]["post_date"])
def open_sites(self):
os.system("start http://www.amazon.de/")
root = Tk()
Gui(root)
Was soll denn diese Zeile bewirken?
Um im Zuge der Wartbarkeit hätte ich RawStrings an den gewissen Stellen verwendet.
Code: Alles auswählen
import FileHandler as FileHandler