K
K
kingslayer2021-07-13 16:09:31
Python
kingslayer, 2021-07-13 16:09:31

Is it bad code?

Help me understand if this code is bad.
What can be improved?

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()

Answer the question

In order to leave comments, you need to log in

2 answer(s)
D
Deleting Account, 2021-07-13
@kingslayer

1. PEP8
2. I understand that ask_color, mouse_motion2, , mouse_motion are used only inside the class - then these methods should be private, not public.
3. mouse_motion1, mouse_motion2, mouse_motion3 - these are very bad names, try to name them so that the name makes it clear what a particular method does, if they do several things - break the method into several.
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)
- I would put this in a separate function that __init__ already calls, although this is optional.

N
Nikita Mokhovikov, 2021-07-13
@mohovoy

Check your code against PEP8 requirements

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question