Programmiertechnik Python

[Challanger]

Hallo

ich hatte hier vor einigen Tgen schon Hilfe bekommen, wobei sich erausstellte das ich mit das Objektoptimierte Programmieren mal anschauen sollte.
Habe jetzt in einem Buch gelesen und einige Videos angeschaut zu dem Thema. Allerdings denke ich das ich nur die Hälfte verstanden habe.
Jedenfalls habe ich nachfolgendes Progamm etwas weiter gesponnen. Es Funktioniert soweit ganz gut.
Ich werde aber das gefühl nicht los das es Programmiertechnisch eine Katastrophe ist.

Schut mal drüber wenn ihr zeit und lust habt

Code: Alles auswählen

from tkinter import *
import tkinter as tk
     
class Fenster(tk.Tk):
        def __init__(self):
            super().__init__()
            self.counter1 = 0                  # zähler für menü
            self.counter2 = 0                  # zähler für menü
            self.kontrast = 100                # wert für kontrast
            self.farbe = 100                   # wert für farbe
            self.xwert = 150                   # X wert für fadenkreuz
            self.ywert = 150                   # Y Wert für Fadenkreuz
            
            self.menue = "                              " , " Gerät AUS " , " Fadenkreuz " , " Farbe Faden. " , " Kontrast "
            self.title("Display")
            self.geometry("580x300")
            zeichner = tk.Canvas(self)
            zeichner.place(x = 150, y = 0)
            
            zeichner.create_line(2,2,318,2)
            zeichner.create_line(318,2,318,260)
            zeichner.create_line(2,260,318,260)
            zeichner.create_line(2,2,2,260)
            zeichner.create_oval(2,2,318,260)
            zeichner.create_line(self.xwert,1,self.xwert,260)
            zeichner.create_line(2,self.ywert,318,self.ywert)
            knopf1 = tk.Button(self, text="Plus", command=self.ausgabeplus)
            knopf2 = tk.Button(self, text="SET", command=self.ausgabeset)
            knopf3 = tk.Button(self, text="Minus", command=self.ausgabeminus)
            knopf4 = tk.Button(self, text="Links", command=self.ausgabelinks)
            knopf5 = tk.Button(self, text="Rechts", command=self.ausgaberechts)
            knopf1.place(x = 50, y = 0 , width=50, height=50)
            knopf2.place(x = 50, y = 50 , width=50, height=50)
            knopf3.place(x = 50, y = 100 , width=50, height=50)
            knopf4.place(x = 0, y = 50 , width=50, height=50)   
            knopf5.place(x = 100, y = 50 , width=50, height=50)

        def label(self):
            label1 = Label(self)
            label1.place(x = 160, y =50 )
            label1.configure(text=self.men[self.b])

        def ausgabelinks(self):
                if self.counter2 == 2:
                        self.xwert = self.xwert -1
                        zeichner = tk.Canvas(self)
                        zeichner.place(x = 150, y = 0)
                        zeichner.create_oval(2,2,318,260)
                        zeichner.create_oval(self.xwert-5,self.ywert-5,self.xwert+5,self.ywert+5)

        def ausgaberechts(self):
                if self.counter2 == 2:
                        self.xwert = self.xwert + 1
                        zeichner = tk.Canvas(self)
                        zeichner.place(x = 150, y = 0)
                        zeichner.create_oval(2,2,318,260)
                        zeichner.create_oval(self.xwert-5,self.ywert-5,self.xwert+5,self.ywert+5)                

        def ausgabeset(self):
            
            label1 = Label(self)
            label1.place(x = 260, y =70 )
            label1.configure(text="                    ")


            if self.counter1 == 0:
                pass
            elif self.counter2 == 1:
                 self.counter2 = 0
                
            elif self.counter2 == 2:
                 self.counter2 = 0

            elif self.counter1 == 1:
                label1 = Label(self)
                label1.place(x = 170, y =50 )
                label1.configure(text="System wird heruntergefahren         ")

            elif self.counter1 == 2:                       # Fadenkreuz
                    self.counter2 = 2                   
                
                    zeichner = tk.Canvas(self)
                    zeichner.place(x = 150, y = 0)
                    zeichner.create_oval(2,2,318,260)
              
                    zeichner.create_oval(self.xwert-5,self.ywert-5,self.xwert+5,self.ywert+5)
                    

            elif self.counter1 == 3:
                label1 = Label(self)
                label1.place(x = 160, y =50 )
                label1.configure(text="                                    ")
            elif self.counter1 == 4:
                self.counter2=1
                
            zeichner = tk.Canvas(self)
            zeichner.place(x = 150, y = 0)    
            zeichner.create_oval(2,2,318,260)
            zeichner.create_line(self.xwert,1,self.xwert,260)
            zeichner.create_line(2,self.ywert,318,self.ywert)
            print(self.counter2)
                
                
           
            
            
     
        def ausgabeplus(self): 

            
            if self.counter1 > 4:
                self.counter1 = 0
            elif self.counter2 == 0:
                self.counter1 = self.counter1 + 1
                if self.counter1 > 4:
                    self.counter1 = 0
            elif self.counter2 == 1:
                self.kontrast = self.kontrast +1
                label1 = Label(self)
                label1.place(x = 260, y =70 )
                label1.configure(text= self.kontrast )
            elif self.counter2 == 2:
                    self.ywert = self.ywert -1
                    zeichner = tk.Canvas(self)
                    zeichner.place(x = 150, y = 0)
                    zeichner.create_oval(2,2,318,260) 
                    zeichner.create_oval(self.xwert-5,self.ywert-5,self.xwert+5,self.ywert+5)

            print(self.menue[self.counter1])
            print(self.kontrast)
            print("Counter1 = ",self.counter1)
            print("Counter2 = ",self.counter2)
            label1 = Label(self)
            label1.place(x = 260, y =50 )
            label1.configure(text=self.menue[self.counter1])
            
     
        def ausgabeminus(self):

            
            if self.counter1 < 0:
                self.counter1 = 4
            elif self.counter2 == 0:
                self.counter1 = self.counter1 - 1
                if self.counter1 < 0:
                    self.counter1 = 4

                
            elif self.counter2 == 1:
                self.kontrast = self.kontrast -1
                label1 = Label(self)
                label1.place(x = 260, y =70 )
                label1.configure(text= self.kontrast )

            elif self.counter2 == 2:
                    self.ywert = self.ywert +1
                    zeichner = tk.Canvas(self)
                    zeichner.place(x = 150, y = 0)
                    zeichner.create_oval(2,2,318,260)
                    zeichner.create_oval(self.xwert-5,self.ywert-5,self.xwert+5,self.ywert+5)    
               
            print(self.menue[self.counter1])
            print(self.kontrast)
            print("Counter1 = ",self.counter1)
            print("Counter2 = ",self.counter2)
            label1 = Label(self)
            label1.place(x = 260, y =50 )
            label1.configure(text=self.menue[self.counter1])
     
def main():
        
        fenster = Fenster()
        
        
        fenster.mainloop()
       
        
       
if __name__ == '__main__':
       
        main()

[BlackJack]

@Challanger: Den Sternchen-Import solltest Du loswerden. Du importierst das Modul ja sogar schon ohne gleich alles in den Namensraum zu kippen. Warum einige Namen auf die eine Weise und Andere auf die andere Weise verwenden?

Das es nur eine grosse Klasse gibt und keine Methode Argumente entgegen nimmt und keine einen Rückgabewert hat, ist ein „code smell“ Das ist ja im Grunde ein komplettes Programm mit quasi-globalen Variablen, weil der komplette Zustand des Programms in diesem einen Objekt ”gekapselt” ist. Und so ziemlich jede Methode scheint auf diese ominösen, numerierten `counter`-Variablen zuzugreifen. Das ist im Quelltext unübersichtich und auch wenn man die GUI laufen lässt ist das nicht wirklich verständlich. Zumal die Ausgabe sich auch merkwürdig verhält. Wenn man beispielsweise „Kontrast“ auswählt (nachdem man verstanden hat wie das Menü funktioniert), dann wird die aktuelle Einstellung gar nicht angezeigt. Erst wenn man sie erhöht oder verringert wird der Wert angezeigt.

Du benutzt Tk falsch. Es werden ständig und immer mehr GUI-Elemente erzeugt die dann per `place()` übereinander gestapelt werden. Das ist ein Riesenspeicherleck und wird mit der Zeit die GUI auch immer langsamer werden lassen weil diese ganzen Elemente alle immer wieder und wieder gezeichnet werden müssen. Man sieht zwar nur das ”oberste”, aber die anderen sind immer noch da und werden jedes mal wenn die GUI aktualisiert werden muss, allesamt von ”hinten nach vorne” gezeichnet. Das normale vorgehen wäre die Elemente nur *einmal* zu erzeugen und dann die Eigenschaften zu verändern, also zum Beispiel den Text eines Labels oder die Position von gezeichneten Canvas-Objekten (wozu man sich deren ID merken müsste).

Die Aufteilung der Logik auf die Methoden ist nicht gut. Jede Methode kennt die ganzen Zustände und Variablen die mit den Knöpfen verändert werden können. Die gleiche Logik um festzustellen in welchem Zustand sich das Programm gerade befindet, ist in mehr als einer Methode im Grunde gleich vorhanden. Daten- und Codewiederholungen vermeidet man als Programmierer, da bei Änderungen mehrere Stellen im Code verändert werden müssen, und zwar gleichartig. Das ist eine Fehlerquelle.

Man könnte das ganze übersichlicher mit ein bisschen mehr OOP lösen. Zum Beispiel den Teil der nichts mit der GUI zu tun hat, also die vier Werte die letztendlich manipuliert werden sollen, in eine eigene Klasse kapseln, und für die einzelnen Menüpunkte und das Menü selbst Objekte mit Methoden, die auf die fünf Schaltflächen reagieren können, verwenden.

Warum überhaupt diese komische Benutzeroberfläche mit den vier Schaltflächen für alles, statt vier Schaltflächen für das Fadenkreuz und jeweils zwei Schaltflächen und ein Eingabefeld für die beiden Zahlenwerte und eine Schaltfläche zum ausschalten?

[BlackJack]

Mal ”aus der Hüfte geschossen” ein Versuch:

Code: Alles auswählen

#!/usr/bin/env python
# coding: utf-8
from __future__ import absolute_import, division, print_function
import Tkinter as tk
from functools import partial


class Coordinate(object):

    def __init__(self, x, y):
        self.x = x
        self.y = y

    def __add__(self, other):
        return Coordinate(self.x + other.x, self.y + other.y)

Coordinate.UP, Coordinate.DOWN, Coordinate.LEFT, Coordinate.RIGHT = [
    Coordinate(0, -1), Coordinate(0, 1), Coordinate(-1, 0), Coordinate(1, 0)
]


class Device(object):
    # 
    # TODO Are there any boundaries for the values that need to be
    #   enforced here?
    # 
    def __init__(self):
        self.contrast = 100
        self.colour = 100
        self.coordinate = Coordinate(150, 150)

    def change_contrast(self, amount):
        self.contrast += amount

    def change_colour(self, amount):
        self.colour += amount

    def change_coordinate(self, delta):
        self.coordinate += delta

    def switch_off(self):
        pass  # TODO Whatever has to happen here.


class BaseController(object):

    IS_MAIN_MENU = False

    def __init__(self, name, device):
        self.name = name
        self.value = None
        self.device = device
        self.previous_controller = None

    def activate(self, current_controller):
        self.previous_controller = current_controller
        return self

    def _on_direction(self):
        pass

    on_plus = on_minus = on_left = on_right = _on_direction

    def on_set(self):
        return self.previous_controller


class NullController(BaseController):

    @staticmethod
    def activate(current_controller):
        return current_controller


class OffSwitchController(BaseController):

    def on_set(self):
        self.device.switch_off()
        BaseController.on_set(self)


class CrossHairController(BaseController):

    def on_plus(self):
        self.device.change_coordinate(Coordinate.UP)

    def on_minus(self):
        self.device.change_coordinate(Coordinate.DOWN)

    def on_left(self):
        self.device.change_coordinate(Coordinate.LEFT)

    def on_right(self):
        self.device.change_coordinate(Coordinate.RIGHT)


class NumberController(BaseController):

    def __init__(self, name, device, attribute_name):
        BaseController.__init__(self, name, device)
        self.attribute_name = attribute_name
        self.change_value = getattr(
            self.device, 'change_' + self.attribute_name
        )

    @property
    def value(self):
        return getattr(self.device, self.attribute_name)

    @value.setter
    def value(self, _value):
        pass

    def on_plus(self):
        self.change_value(1)

    def on_minus(self):
        self.change_value(-1)


class MenuController(BaseController):

    IS_MAIN_MENU = True
    UP, DOWN = -1, 1

    def __init__(self, name, device, items):
        BaseController.__init__(self, name, device)
        self.items = items
        self.index = 0
        self._change_menu_item(0)

    @property
    def name(self):
        return self.items[self.index].name

    @name.setter
    def name(self, _value):
        pass

    def _change_menu_item(self, direction):
        self.index = (self.index + direction) % len(self.items)
        self.previous_controller = self.items[self.index]

    def on_plus(self):
        self._change_menu_item(self.UP)

    def on_minus(self):
        self._change_menu_item(self.DOWN)


class ControlButtons(tk.Frame):

    def __init__(self, parent, device, menu_controller):
        tk.Frame.__init__(self, parent)
        self.device = device
        self.controller = menu_controller
        for text, command, row, column in [
            ('Plus', partial(self.on_direction, 'on_plus'), 0, 1),
            ('Minus', partial(self.on_direction, 'on_minus'), 2, 1),
            ('Left', partial(self.on_direction, 'on_left'), 1, 0),
            ('Right', partial(self.on_direction, 'on_right'), 1, 2),
            ('SET', self.on_set, 1, 1),
        ]:
            tk.Button(
                self, text=text, command=command, width=3, height=3
            ).grid(row=row, column=column)

    @property
    def is_main_menu(self):
        return self.controller.IS_MAIN_MENU

    @property
    def menu_name(self):
        return self.controller.name

    @property
    def menu_value(self):
        value = self.controller.value
        return '' if value is None else str(value)

    def on_direction(self, controller_method_name):
        getattr(self.controller, controller_method_name)()
        self.event_generate('<<Refresh>>')

    def on_set(self):
        previous_controller = self.controller
        self.controller = self.controller.on_set().activate(previous_controller)
        self.event_generate('<<Refresh>>')


class DisplayUI(tk.Canvas):
    
    def __init__(self, parent, controls, device, size=(320, 262)):
        self.width, self.height = size
        tk.Canvas.__init__(
            self,
            parent,
            width=self.width,
            height=self.height,
            highlightthickness=0,
        )
        self.controls = controls
        self.device = device
        padding = 2
        self.create_rectangle(
            padding, padding, self.width - padding, self.height - padding
        )
        self.create_oval(
            padding, padding, self.width - padding, self.height - padding
        )
        self.cross_hair_horizontal = self.create_line(0, 0, self.width, 0)
        self.cross_hair_vertical = self.create_line(0, 0, 0, self.height)
        self.menu_name = self.create_text(self.width // 2, 50)
        self.menu_value = self.create_text(self.width // 2, 70)
        self.controls.bind('<<Refresh>>', self.refresh)
        self.refresh()

    def refresh_crosshair(self):
        coordinate = self.device.coordinate
        self.coords(
            self.cross_hair_horizontal,
            0,
            coordinate.y,
            self.width,
            coordinate.y
        )
        self.coords(
            self.cross_hair_vertical, coordinate.x, 0, coordinate.x, self.height
        )
    
    def refresh_menu_texts(self):
        self.itemconfig(
            self.menu_name,
            text=self.controls.menu_name,
            fill='black' if self.controls.is_main_menu else 'red',
        )
        self.itemconfig(self.menu_value, text=self.controls.menu_value)

    def refresh(self, _event=None):
        self.refresh_crosshair()
        self.refresh_menu_texts()


def main():
    device = Device()
    menu_controller = MenuController(
        None,
        device,
        [
            NullController('', device),
            OffSwitchController('Gerät AUS', device),
            CrossHairController('Fadenkreuz', device),
            NumberController('Farbe faden', device, 'colour'),
            NumberController('Kontrast', device, 'contrast'),
        ]
    )
    root = tk.Tk()
    root.title('Display')
    controls = ControlButtons(root, device, menu_controller)
    controls.pack(side=tk.LEFT)
    display = DisplayUI(root, controls, device)
    display.pack(side=tk.LEFT)
    root.mainloop()


if __name__ == '__main__':
    main()