Das deutsche Python-Forum

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

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?

@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.

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.

@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?

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?

Code: Alles auswählen

import FileHandler as FileHandler

Um im Zuge der Wartbarkeit hätte ich RawStrings an den gewissen Stellen verwendet.

Das deutsche Python-Forum

Kritik am code

Kritik am code

Re: Kritik am code

Re: Kritik am code

Re: Kritik am code

Re: Kritik am code

Re: Kritik am code

Re: Kritik am code