Disclaimer: you should not be using eval that said I am not going to be removing it from the code as you can work out the correct options on your own. I will be reviewing the overall code issues. Just know eval is evil! :D
Disclaimer: you should not be using eval that said I am not going to be removing it from the code as you can work out the correct options on your own. I will be reviewing the overall code issues. Just know eval is evil! :D
I will keep each one of your files separate but honestly I do not think it is necessary to separate the main window file from the frame datedata.
Just for fun here is how I would have build this calc. Its a small enough program I think most if not all is fine in a single class. Also by placing everything in a single class we can avoid a lot of the back and forth going on and keep our code simple. By doing this we took your roughly 180+ lines of code and reduced them to around 80+ lines of code.
My example:
import tkinter as tk
class Main(tk.Tk):
def __init__(self):
super().__init__()
self.geometry("640x640")
self.columnconfigure(0, weight=1)
self.rowconfigure(1, weight=1)
self.aggregator = ''
self.txt = tk.StringVar()
self.bind("<Key>", self.key_handler)
self.bind("<Return>", self.equal)
dis_frame = tk.Frame(self)
dis_frame.grid(row=0, column=0, sticky="new")
btn_frame = tk.Frame(self)
btn_frame.grid(row=1, column=0, sticky="nsew")
dis_frame.configure(bg="cyan", height=5)
dis_frame.columnconfigure(0, weight=1)
for x in range(0, 5):
btn_frame.rowconfigure(x, weight=1)
if x < 4:
btn_frame.columnconfigure(x, weight=1)
self.display = tk.Label(dis_frame, textvariable=self.txt, font=15,
bg="#bebebe", relief="groove", bd=5, height=5)
self.display.grid(row=0, column=0, sticky="nsew")
pad = 15
r = 0
c = 0
for i in range(10):
if i == 0:
tk.Button(btn_frame, text=i, padx=pad, pady=pad,
command=lambda n=i: self.num(n)).grid(row=3, column=1, sticky="nswe")
else:
tk.Button(btn_frame, text=i, padx=pad, pady=pad,
command=lambda n=i: self.num(n)).grid(row=r, column=c, sticky="nswe")
if c == 2:
c = 0
r += 1
else:
c += 1
for i in [["-", 1, 3], ["*", 2, 3], ["/", 3, 3], ["(", 3, 0],
[")", 3, 2], [".", 4, 2], ["+", 0, 3], ["=", 4, 3], ["CLEAR", 4, 0]]:
if i[0] == 'CLEAR':
tk.Button(btn_frame, text=i[0], padx=pad, pady=pad,
command=self.clear).grid(row=i[1], column=i[2], columnspan=2, sticky="nsew")
elif i[0] == '=':
tk.Button(btn_frame, text=i[0], padx=pad, pady=pad,
command=self.equal).grid(row=i[1], column=i[2], sticky="nsew")
else:
tk.Button(btn_frame, text=i[0], padx=pad, pady=pad,
command=lambda v=i[0]: self.num(v)).grid(row=i[1], column=i[2], sticky="nsew")
def key_handler(self, event):
self.num(event.char)
def num(self, n):
self.aggregator += str(n)
self.txt.set(self.aggregator)
def equal(self, event=None):
try:
total = str(eval(self.aggregator))
self.txt.set(total)
self.aggregator = total
except ZeroDivisionError:
self.txt.set("Error: Divisão por zero")
except:
self.txt.set("Unexpected error")
raise
def clear(self):
self.txt.set("Clear")
self.aggregator = ''
Main().mainloop()
I will keep each one of your files separate but honestly I do not think it is necessary to separate the main window file from the frame date.
I will keep each one of your files separate but honestly I do not think it is necessary to separate the main window file from the frame data.
Just for fun here is how I would have build this calc. Its a small enough program I think most if not all is fine in a single class. Also by placing everything in a single class we can avoid a lot of the back and forth going on and keep our code simple. By doing this we took your roughly 180+ lines of code and reduced them to around 80+ lines of code.
My example:
import tkinter as tk
class Main(tk.Tk):
def __init__(self):
super().__init__()
self.geometry("640x640")
self.columnconfigure(0, weight=1)
self.rowconfigure(1, weight=1)
self.aggregator = ''
self.txt = tk.StringVar()
self.bind("<Key>", self.key_handler)
self.bind("<Return>", self.equal)
dis_frame = tk.Frame(self)
dis_frame.grid(row=0, column=0, sticky="new")
btn_frame = tk.Frame(self)
btn_frame.grid(row=1, column=0, sticky="nsew")
dis_frame.configure(bg="cyan", height=5)
dis_frame.columnconfigure(0, weight=1)
for x in range(0, 5):
btn_frame.rowconfigure(x, weight=1)
if x < 4:
btn_frame.columnconfigure(x, weight=1)
self.display = tk.Label(dis_frame, textvariable=self.txt, font=15,
bg="#bebebe", relief="groove", bd=5, height=5)
self.display.grid(row=0, column=0, sticky="nsew")
pad = 15
r = 0
c = 0
for i in range(10):
if i == 0:
tk.Button(btn_frame, text=i, padx=pad, pady=pad,
command=lambda n=i: self.num(n)).grid(row=3, column=1, sticky="nswe")
else:
tk.Button(btn_frame, text=i, padx=pad, pady=pad,
command=lambda n=i: self.num(n)).grid(row=r, column=c, sticky="nswe")
if c == 2:
c = 0
r += 1
else:
c += 1
for i in [["-", 1, 3], ["*", 2, 3], ["/", 3, 3], ["(", 3, 0],
[")", 3, 2], [".", 4, 2], ["+", 0, 3], ["=", 4, 3], ["CLEAR", 4, 0]]:
if i[0] == 'CLEAR':
tk.Button(btn_frame, text=i[0], padx=pad, pady=pad,
command=self.clear).grid(row=i[1], column=i[2], columnspan=2, sticky="nsew")
elif i[0] == '=':
tk.Button(btn_frame, text=i[0], padx=pad, pady=pad,
command=self.equal).grid(row=i[1], column=i[2], sticky="nsew")
else:
tk.Button(btn_frame, text=i[0], padx=pad, pady=pad,
command=lambda v=i[0]: self.num(v)).grid(row=i[1], column=i[2], sticky="nsew")
def key_handler(self, event):
self.num(event.char)
def num(self, n):
self.aggregator += str(n)
self.txt.set(self.aggregator)
def equal(self, event=None):
try:
total = str(eval(self.aggregator))
self.txt.set(total)
self.aggregator = total
except ZeroDivisionError:
self.txt.set("Error: Divisão por zero")
except:
self.txt.set("Unexpected error")
raise
def clear(self):
self.txt.set("Clear")
self.aggregator = ''
Main().mainloop()
Ok so quick answer to fix the main problem is to add a new argument to all functions in calculadora.py lets call this argument window because we are passing the the root window to each function.
Then you need to build the root window as a class with class attributes. This way your functions in calculadora can actually update the the fields.
Once we changed those 2 parts we need to pass that window to those functions from the frame_botoes.py buttons so we will update those buttons as well.
Updated window.py:
import tkinter as tk import frame_display import frame_botoes
class Main(tk.Tk):
def __init__(self):
super().__init__()
self.geometry("640x640")
self.visor = frame_display.DisplayContainer(self)
self.numeros = frame_botoes.ButtonsContainer(self)
Main().mainloop()
Updated calculadora.py:
agregator = ""
result = ""
def pressNumber(num, window):
global agregator
global result
agregator = agregator + str(num)
result = agregator
window.visor.updateTextDisplay(result)
def pressEqual(window):
try:
global agregator
total = str(eval(agregator))
window.visor.updateTextDisplay(total)
agregator = ""
except ZeroDivisionError:
window.visor.updateTextDisplay("Erro: Divisão por zero")
agregator = ""
except:
window.visor.updateTextDisplay("Error")
agregator = ""
def pressClear(window):
global agregator
agregator = ""
window.visor.updateTextDisplay("Clear")
Updated frame_botoes.py:
import tkinter as tk
from tkinter import Frame
import calculadora
class ButtonsContainer(Frame):
def __init__(self , root):
Frame.__init__(self, root)
self.parent = root
self.configure(bg="yellow")
self.parent.bind("<Key>", self.keyHandler)
self.parent.bind("<Return>", self.returnKeyHandler)
# Layout ButtonsContainer
self.grid(row=1 , column=0 , sticky ="nsew")
self.parent.rowconfigure(1, weight=1)
self.parent.columnconfigure(0, weight=1)
# Call ButtonsContainer widgets creation
self.createWidgets()
# Create widgets for ButtonsContainer
def createWidgets(self):
button_padx = 15
button_pady = 15
self.button_1 = tk.Button(self, text="1", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(1, self.parent))
self.button_2 = tk.Button(self, text="2", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(2, self.parent))
self.button_3 = tk.Button(self, text="3", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(3, self.parent))
self.button_4 = tk.Button(self, text="4", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(4, self.parent))
self.button_5 = tk.Button(self, text="5", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(5, self.parent))
self.button_6 = tk.Button(self, text="6", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(6, self.parent))
self.button_7 = tk.Button(self, text="7", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(7, self.parent))
self.button_8 = tk.Button(self, text="8", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(8, self.parent))
self.button_9 = tk.Button(self, text="9", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(9, self.parent))
self.button_0 = tk.Button(self, text="0", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(0, self.parent))
self.button_open_parens = tk.Button(self, text="(", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber("(", self.parent))
self.button_close_parens = tk.Button(self, text=")", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(")", self.parent))
self.button_dot = tk.Button(self, text=".", padx= button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(".", self.parent))
self.button_plus = tk.Button(self, text="+", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber("+", self.parent))
self.button_minus = tk.Button(self, text="-", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber("-", self.parent))
self.button_multiply = tk.Button(self, text="*", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber("*", self.parent))
self.button_divide = tk.Button(self, text="/", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber("/", self.parent))
self.button_equal = tk.Button(self, text="=", padx=button_padx, pady=button_pady, command=calculadora.pressEqual(self.parent))
self.button_clear = tk.Button(self, text="CLEAR", padx=button_padx, pady=button_pady, command=calculadora.pressClear(self.parent))
# Layout widgets for ButtonsContainer
self.button_1.grid(row=0, column=0, sticky="nswe")
self.button_2.grid(row=0, column=1, sticky="nswe")
self.button_3.grid(row=0, column = 2, sticky="nswe")
self.button_4.grid(row=1, column=0, sticky="nswe")
self.button_5.grid(row=1, column=1, sticky="nswe")
self.button_6.grid(row=1, column=2, sticky="nswe")
self.button_7.grid(row=2, column=0, sticky="nswe")
self.button_8.grid(row=2, column=1, sticky="nswe")
self.button_9.grid(row=2, column=2, sticky="nswe")
self.button_open_parens.grid(row=3, column=0, sticky="nswe")
self.button_close_parens.grid(row=3, column=2, sticky="nswe")
self.button_0.grid(row=3, column=1, sticky="nswe")
self.button_dot.grid(row=4, column=2, sticky="nswe")
self.button_plus.grid(row=0 , column=3, sticky="nswe")
self.button_minus.grid(row=1 , column=3, sticky="nswe")
self.button_multiply.grid(row=2 , column=3, sticky="nswe")
self.button_divide.grid(row=3 , column=3, sticky="nswe")
self.button_equal.grid(row=4 , column=3, sticky="nswe")
self.button_clear.grid(row=4 , columnspan=2, sticky="nswe")
for x in range(0,5):
self.rowconfigure(x, weight=1)
for i in range(0, 4):
self.columnconfigure(i, weight=1)
#Bind keyboard events
def keyHandler(self, event):
calculadora.pressNumber(event.char, self.parent)
#Bind Return key
def returnKeyHandler(self, event):
calculadora.pressEqual()
Now that the quick fix is dealt with its time to go in depth as to the other formatting issues and PEP8 changes we should make.
I will keep each one of your files separate but honestly I do not think it is necessary to separate the main window file from the frame date.
1st: I would like to address is PEP8 standards. Personally I think you should use CamelCase for Class names and lowercase_with_underscores for functions/methods.
2nd: Lets look at your buttons in frame_botoes. You should probably be generating your buttons with loops so we can keep the code short and clean. I have 2 examples here. One uses simple counting for the layout and the other uses a list with grid values for placement.
3rd: We should avoid using global so lets convert your calculadora functions into a class that we use with class attribute to manage the aggregator.
4th: You only need self. prefix for a variable that will be changed later in the class outside of the method it is generated in. So for all your buttons we can remove this prefix. At the same time we don't need to name them as we are generating them from a loop. Naming doesn't help us here as the layout is simple enough and we are not changing the buttons later.
5th: We do not need from tkinter import Frame as you are already using import tkinter as tk so we can simply call tk.Frame or any other widget for that matter where it is needed.
With some general clean up and the things I mentioned above here is your modified code:
New window.py:
import tkinter as tk
import frame_display
import frame_botoes
class Main(tk.Tk):
def __init__(self):
super().__init__()
self.geometry("640x640")
self.columnconfigure(0, weight=1)
self.rowconfigure(1, weight=1)
self.visor = frame_display.DisplayContainer().grid(row=0, column=0, sticky="new")
self.numeros = frame_botoes.ButtonsContainer().grid(row=1, column=0, sticky="nsew")
Main().mainloop()
New calculadora.py:
class Press:
def __init__(self, master):
self.master = master
self.aggregator = ''
def num(self, n):
self.aggregator += str(n)
self.master.visor.update_text_display(self.aggregator)
def equal(self, _):
try:
total = str(eval(self.aggregator))
self.aggregator = ''
self.master.visor.text_display.set(total)
except ZeroDivisionError:
self.master.visor.text_display.set("Error: Divisão por zero")
except:
self.master.visor.text_display.set("Unexpected error")
raise
def clear(self):
self.master.visor.text_display.set("Clear")
New frame_display.py:
import tkinter as tk
class DisplayContainer(tk.Frame):
def __init__(self):
super().__init__()
self.configure(bg="cyan", height=5)
self.columnconfigure(0, weight=1)
self.txt = tk.StringVar()
label_display = tk.Label(self, textvariable=self.txt, font=15, bg="#bebebe", relief="groove", bd=5, height=5)
label_display.grid(row=0, column=0, sticky="nsew")
def update_text_display(self, text):
self.text_display.set(text)
New frame_botoes.py:
import tkinter as tk
import calculadora
class ButtonsContainer(tk.Frame):
def __init__(self):
super().__init__()
self.configure(bg="yellow")
self.screen = calculadora.Press(self.master)
self.master.bind("<Key>", self.key_handler)
self.master.bind("<Return>", self.screen.equal)
for x in range(0, 5):
self.rowconfigure(x, weight=1)
if x < 4:
self.columnconfigure(x, weight=1)
pad = 15
r = 0
c = 0
for i in range(10):
if i == 0:
tk.Button(self, text=i, padx=pad, pady=pad,
command=lambda n=i: self.screen.num(n)).grid(row=3, column=1, sticky="nswe")
else:
tk.Button(self, text=i, padx=pad, pady=pad,
command=lambda n=i: self.screen.num(n)).grid(row=r, column=c, sticky="nswe")
if c == 2:
c = 0
r += 1
else:
c += 1
for i in [["-", 1, 3], ["*", 2, 3], ["/", 3, 3], ["(", 3, 0],
[")", 3, 2], [".", 4, 2], ["+", 0, 3], ["=", 4, 3], ["CLEAR", 4, 0]]:
if i[0] == 'CLEAR':
tk.Button(self, text=i[0], padx=pad, pady=pad,
command=self.screen.clear).grid(row=i[1], column=i[2], columnspan=2, sticky="nsew")
elif i[0] == '=':
tk.Button(self, text=i[0], padx=pad, pady=pad,
command=self.screen.equal).grid(row=i[1], column=i[2], sticky="nsew")
else:
tk.Button(self, text=i[0], padx=pad, pady=pad,
command=lambda v=i[0]: self.screen.num(v)).grid(row=i[1], column=i[2], sticky="nsew")
def key_handler(self, event):
self.screen.num(event.char)
If you have any questions let me know :D