Tkinter Formatierungsfehler bei Scrolleiste, Checkboxen, usw.
- __blackjack__
- User
- Beiträge: 14341
- Registriert: Samstag 2. Juni 2018, 10:21
- Wohnort: 127.0.0.1
- Kontaktdaten:
@rlinke81: Ein paar Anmerkungen zum Quelltext:
Das Hauptprogramm sollte in einer Funktion stecken. Üblicherweise `main()` genannt.
`config_file` ist gar keine Datei, sondern ein Datei*name*. Bei etwas das `file` heisst, würde man Methoden wie `close()` und `read()` oder `write()` erwarten.
`load_config()` wäre besser eine Funktion die den Dateinamen als Argument bekommt. Der Code darin hat ja nicht wirklich etwas mit der GUI zu tun. Analog würde eine `save_config()`-Funktion Sinn machen, die den nicht-GUI-Teil des speicherns enthält.
Beim öffnen von Textdateien sollte man immer explizit die Kodierung angeben. Bei JSON ist das UTF-8.
`config_data` und `config_values` sind keine guten Namen. Selbst wenn man weiss, dass das eine die tatsächliche Konfiguration enthält und das andere die Zuordnung zu GUI-Elementen, muss raten was denn nun was ist. `config_data` würde ich `configuration` oder `key_to_value` nennen, und `config_values` eher `key_to_var`. Dann weiss man schon anhand des Namens was da als Schlüssel und Werte jeweils drin ist.
Das Auslagern der Widget-Erzeugung in eine eigene Methode macht nicht wirklich Sinn.
Statt `row` manuell am Ende eines sehr langen Schleifenkörpers manuell hoch zu zählen, würde man da besser `enumerate()` verwenden.
Wenn es Konstanten für Zeichenketten mit besonderer Bedeutung als Argumente im `tkinter`-Modul gibt, sollte man diese Konstanten verwenden, statt die als literale Zeichenkette hin zu schreiben.
Man muss nicht alles an einen Namen binden, wenn man das nicht wirklich noch mal braucht.
`fov_radius` wird definiert, aber nicht verwendet.
Das da im ``else`` alle vorher nicht behandelten Werte als Zeichenketten behandelt werden, beziehungsweise dann später bei der Speichermethode in den Zeichenketten noch mal nach einer Liste geschaut wird ist nicht wirklich robust. Insbesondere auch nicht Verarbeitung der Liste mit Zeichenkettenoperationen. Das ist ziemlich gruselig. Wenn der Benutzer bei einer Zeichenkette Ziffern eingibt, wird das beim Speichern dann auch tatsächlich eine Zahl, auch wenn das eigentlich eine Zeichenkette war. Andere eingelesene Werte wie `None` oder Wörterbücher werden als Zeichenketten gespeichert, statt als `null` oder JSON-Objekt. Sollten solche Werte nicht vorkommen (dürfen), wäre es trotzdem sinnvoll da wenigstens drauf zu testen und die unverändert durchzureichen. Eventuell mit einer Warnung für den Benutzer, dass da etwas unerwartetes in der Datei ist.
Alle Zweige enthalten die gleiche Zeile um die Variable (oder das `Entry`) dem Wörterbuch hinzuzufügen. Diese Zeile kann man also auch *einmal* hinter die Schleife schreiben.
Es macht hier keinen Sinn für das Speichern eine Kopie der Daten anzulegen.
Test auf "." ist kein robuster Test ob eine Zeichenkette eine Darstellung einer Gleitkommazahl ist. "1e6" hat beispielsweise keinen "." ist aber mit `float()`, dafür aber nicht mit `int()` umwandelbar. Weder für `int()` noch für `float()` muss man vorher `strip()` aufrufen.
Die ganzen Unterscheidungen sollten nicht sein, wenn man hier OOP einsetzt und `Variable`-Objekte für jeden Typ hat, die wissen wie der Inhalt umgewandelt werden muss.
Die Fenstergrösse sollte man nicht in absolten Pixeln vorgeben. Die ergibt sich aus dem Inhalt.
Ungetestet:
Das Hauptprogramm sollte in einer Funktion stecken. Üblicherweise `main()` genannt.
`config_file` ist gar keine Datei, sondern ein Datei*name*. Bei etwas das `file` heisst, würde man Methoden wie `close()` und `read()` oder `write()` erwarten.
`load_config()` wäre besser eine Funktion die den Dateinamen als Argument bekommt. Der Code darin hat ja nicht wirklich etwas mit der GUI zu tun. Analog würde eine `save_config()`-Funktion Sinn machen, die den nicht-GUI-Teil des speicherns enthält.
Beim öffnen von Textdateien sollte man immer explizit die Kodierung angeben. Bei JSON ist das UTF-8.
`config_data` und `config_values` sind keine guten Namen. Selbst wenn man weiss, dass das eine die tatsächliche Konfiguration enthält und das andere die Zuordnung zu GUI-Elementen, muss raten was denn nun was ist. `config_data` würde ich `configuration` oder `key_to_value` nennen, und `config_values` eher `key_to_var`. Dann weiss man schon anhand des Namens was da als Schlüssel und Werte jeweils drin ist.
Das Auslagern der Widget-Erzeugung in eine eigene Methode macht nicht wirklich Sinn.
Statt `row` manuell am Ende eines sehr langen Schleifenkörpers manuell hoch zu zählen, würde man da besser `enumerate()` verwenden.
Wenn es Konstanten für Zeichenketten mit besonderer Bedeutung als Argumente im `tkinter`-Modul gibt, sollte man diese Konstanten verwenden, statt die als literale Zeichenkette hin zu schreiben.
Man muss nicht alles an einen Namen binden, wenn man das nicht wirklich noch mal braucht.
`fov_radius` wird definiert, aber nicht verwendet.
Das da im ``else`` alle vorher nicht behandelten Werte als Zeichenketten behandelt werden, beziehungsweise dann später bei der Speichermethode in den Zeichenketten noch mal nach einer Liste geschaut wird ist nicht wirklich robust. Insbesondere auch nicht Verarbeitung der Liste mit Zeichenkettenoperationen. Das ist ziemlich gruselig. Wenn der Benutzer bei einer Zeichenkette Ziffern eingibt, wird das beim Speichern dann auch tatsächlich eine Zahl, auch wenn das eigentlich eine Zeichenkette war. Andere eingelesene Werte wie `None` oder Wörterbücher werden als Zeichenketten gespeichert, statt als `null` oder JSON-Objekt. Sollten solche Werte nicht vorkommen (dürfen), wäre es trotzdem sinnvoll da wenigstens drauf zu testen und die unverändert durchzureichen. Eventuell mit einer Warnung für den Benutzer, dass da etwas unerwartetes in der Datei ist.
Alle Zweige enthalten die gleiche Zeile um die Variable (oder das `Entry`) dem Wörterbuch hinzuzufügen. Diese Zeile kann man also auch *einmal* hinter die Schleife schreiben.
Es macht hier keinen Sinn für das Speichern eine Kopie der Daten anzulegen.
Test auf "." ist kein robuster Test ob eine Zeichenkette eine Darstellung einer Gleitkommazahl ist. "1e6" hat beispielsweise keinen "." ist aber mit `float()`, dafür aber nicht mit `int()` umwandelbar. Weder für `int()` noch für `float()` muss man vorher `strip()` aufrufen.
Die ganzen Unterscheidungen sollten nicht sein, wenn man hier OOP einsetzt und `Variable`-Objekte für jeden Typ hat, die wissen wie der Inhalt umgewandelt werden muss.
Die Fenstergrösse sollte man nicht in absolten Pixeln vorgeben. Die ergibt sich aus dem Inhalt.
Ungetestet:
Code: Alles auswählen
#!/usr/bin/env python3
import json
import math
import tkinter as tk
from ast import literal_eval
from tkinter import filedialog
from tkinter.messagebox import showwarning
def load_config(filename):
with open(filename, "r", encoding="utf-8") as file:
return json.load(file)
def save_config(data, filename):
with open(filename, "w", encoding="utf-8") as file:
json.dump(data, file)
class NumberVar(tk.DoubleVar):
def get(self):
result = tk.DoubleVar.get(self)
return int(result) if result.is_integer() else result
class ListVar(tk.StringVar):
def get(self):
try:
result = literal_eval(tk.StringVar.get(self))
except SyntaxError as error:
raise ValueError("can not parse literal") from error
if not isinstance(result, list):
raise ValueError(f"expected list, got {type(result)}: {result!r}")
return result
class ConfigEditor(tk.Tk):
def __init__(self, config_filename):
super().__init__()
self.title("Config Editor")
self.config_filename = config_filename
self.key_to_value = load_config(self.config_filename)
self.key_to_var = {}
scrollable_frame = tk.Frame(self)
row_index = None
warnings = []
for row_index, (key, value) in enumerate(self.key_to_value.items()):
tk.Label(scrollable_frame, text=key).grid(
row=row_index, column=0, sticky=tk.W
)
if key == "Path":
var = tk.StringVar(value=value)
tk.Entry(scrollable_frame, textvariable=var).grid(
row=row_index, column=1
)
tk.Button(
scrollable_frame,
text="Pfad auswählen",
command=self.select_path,
).grid(row=row_index, column=2)
elif key == "Region":
var = tk.StringVar(value=value)
tk.Entry(scrollable_frame, textvariable=var).grid(
row=row_index, column=1
)
#
# Canvas zum Anzeigen des FOV hinzufügen.
#
fov_canvas = tk.Canvas(
scrollable_frame, width=200, height=200, bg="white"
)
fov_canvas.grid(row=row_index, column=2, padx=10)
fov_angle = math.radians(int(value))
fov_start = math.degrees(math.radians(180) - fov_angle / 2)
fov_end = math.degrees(math.radians(180) + fov_angle / 2)
fov_canvas.create_arc(
50,
50,
150,
150,
start=fov_start,
extent=fov_angle,
style=tk.PIESLICE,
fill="yellow",
)
fov_canvas.create_arc(
50,
50,
150,
150,
start=fov_start,
extent=0.5,
style=tk.PIESLICE,
width=2,
outline="black",
)
fov_canvas.create_arc(
50,
50,
150,
150,
start=fov_end - 0.5,
extent=0.5,
style=tk.PIESLICE,
width=2,
outline="black",
)
elif isinstance(value, bool):
var = tk.BooleanVar(value=value)
tk.Checkbutton(scrollable_frame, variable=var).grid(
row=row_index, column=1
)
elif isinstance(value, list):
var = ListVar(value=value)
tk.Entry(scrollable_frame, variable=var).grid(
row=row_index, column=1
)
elif isinstance(value, (int, float)):
var = NumberVar(value=value)
tk.Entry(scrollable_frame, variable=var).grid(
row=row_index, column=1
)
else:
var = None
warnings.append(
f"value for {key!r} has unexpected type {type(value)}"
)
self.key_to_var[key] = var
if row_index is None:
showwarning("Attention", "The config file was empty!")
self.quit()
else:
tk.Button(
scrollable_frame, text="Save Config", command=self.save_config
).grid(row=row_index, column=0, columnspan=3)
scrollable_frame.pack(side=tk.LEFT, fill=tk.BOTH, expand=True)
if warnings:
showwarning("Unexpected type(s)", "\n".join(warnings))
def save_config(self):
self.key_to_value.update(
(key, var.get())
for key, var in self.key_to_var.items()
if var is not None
)
save_config(self.key_to_value, self.config_filename)
print("Config saved successfully!")
def select_path(self):
filename = filedialog.askopenfilename()
if filename:
self.key_to_var["Path"].set(filename)
def main():
app = ConfigEditor("config.json")
app.mainloop()
if __name__ == "__main__":
main()“It is easier to optimize correct code than to correct optimized code.” — Bill Harlan
