Плохой ли это код?

Question

[kingslayer]

Помогите понять, плохо ли, этот код.
Что можно улучшить?

import tkinter as tk
from tkinter.colorchooser import askcolor
from functools import partial

class App(tk.Tk):
    def __init__(self):
        super().__init__()
        self.movefrom = []
        self.moveto = []
        self.geometry('500x500')
        self.color = None
        self.canvas = tk.Canvas(self, bg='white')
        self.coordinates = tk.Label(self)
        self.prnt_color = tk.Label(self)
        self.button_color = tk.Button(self, text='choose_color', 
                                command=partial(self.ask_color, "fg"))
        
        self.canvas.bind('<Button-1>', self.mouse_motion)
        self.canvas.bind('<Button-3>', self.mouse_motion2)
        
        self.canvas.pack()
        self.coordinates.pack()
        self.prnt_color.pack()
        self.button_color.pack(padx=20, pady=20)
    
    def mouse_motion(self, event):
        self.movefrom.append(event.x)
        self.movefrom.append(event.y)
        text = f'x={event.x}, y={event.y}'
        self.coordinates.config(text=text)
    
    def mouse_motion2(self, event):
        self.moveto.append(event.x)
        self.moveto.append(event.y)
        text = f'x={event.x}, y={event.y}'
        self.coordinates.config(text=text)
        self.canvas.create_line(self.movefrom[0], self.movefrom[1], 
                                self.moveto[0], self.moveto[1],
                                fill=self.color, width=10)
    
        self.movefrom = []
        self.moveto = []
        
    
    def ask_color(self, option):
        ask = askcolor()[1]
        print('выбран цвет {ask}')
        self.prnt_color.config(text=ask)
        self.color = ask
        

if __name__ == '__main__':
    app = App()
    app.mainloop()

Comments

[Pavel Shvedov]

В чем профит сокращать print до prnt?

[@ _ @]

Под пиво пойдет

[kingslayer]

Pavel Shvedov, только это, остальное все норм?

[Pavel Shvedov]

kingslayer, хз я дальше не читал

[Дмитрий]

Этот кусок кода мало что показывает

[fdroid]

Pavel Shvedov, счс тк мдн.

Answer 1

[When you dont want to litter on stackoverflow]

1. PEP8
2. Я так понимаю что ask_color, mouse_motion2, , mouse_motion используются только внутри класса - тогда эти методы должны быть приватными, а не публичными.
3. mouse_motion1, mouse_motion2, mouse_motion3 - эти очень плохие названия попробуй назвать их так, чтобы по названию было понятно что делает конкретный метод, если они делают несколько вещей - разбей метод на несколько.
4.

self.canvas.bind('<Button-1>', self.mouse_motion)
        self.canvas.bind('<Button-3>', self.mouse_motion2)
        
        self.canvas.pack()
        self.coordinates.pack()
        self.prnt_color.pack()
        self.button_color.pack(padx=20, pady=20)

- я бы вынес это в одельную функцию, которою уже вызывает __init__, хотя это опционально.

Comments

[kingslayer]

немного не понимаю, а как это вынести в отдельную функцию?

[When you dont want to litter on stackoverflow]

kingslayer,

class App: 
        def __init__(self):
              ....
              self.__pack()

        def __pack(self):
             self.canvas.pack()
             self.coordinates.pack()
             self.prnt_color.pack()
             self.button_color.pack(padx=20, pady=20)

С именованием можешь поиграться

Answer 2

[Никита Моховиков]

Проверь свой код на соответствие требованиям PEP8