Disclaimer: you should not be usingeval 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 knoweval is evil! :D

Ok so quick answer to fix the main problem is to add a new argument to all functions incalculadora.py lets call this argumentwindow 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 theframe_botoes.py buttons so we will update those buttons as well.

Updatedwindow.py:

import tkinter as tkimport frame_displayimport 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()

Updatedcalculadora.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")

Updatedframe_botoes.py:

import tkinter as tkfrom tkinter import Frameimport calculadoraclass 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 data.

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 inframe_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 usingglobal so lets convert your calculadora functions into a class that we use with class attribute to manage theaggregator.

4th: You only needself. 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 needfrom tkinter import Frame as you are already usingimport tkinter as tk so we can simply calltk.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:

Newwindow.py:

import tkinter as tkimport frame_displayimport frame_botoesclass 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()

Newcalculadora.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")

Newframe_display.py:

import tkinter as tkclass 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)

Newframe_botoes.py:

import tkinter as tkimport calculadoraclass 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

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

• 1
  \$\begingroup\$@BrunoLemos it is super convenient and that is why it is evil :D It can make your code vulnerable to attack.\$\endgroup\$

  Mike - SMT

  – Mike - SMT

  2019-05-07 20:44:48 +00:00

  CommentedMay 7, 2019 at 20:44
• 1
  \$\begingroup\$@BrunoLemos yes it is vulnerable to injection, it is also slow compared to better methods and it can make debugging issue difficult.\$\endgroup\$

  Mike - SMT

  – Mike - SMT

  2019-05-08 13:54:51 +00:00

  CommentedMay 8, 2019 at 13:54
• 1
  \$\begingroup\$@BrunoLemos it doesnt hurt to have the display separate. You may want to keep them separate just in case you want to make a transforming calculator. Like switching between normal and scientific mode. If you have no need to move the display or other widgets later then the extra frame is not needed but doesnt really cause any issues here.\$\endgroup\$

  Mike - SMT

  – Mike - SMT

  2019-05-08 14:25:34 +00:00

  CommentedMay 8, 2019 at 14:25
• 1
  \$\begingroup\$@BrunoLemos You don't want to over write your code. If there is not a good reason to build a function all you are doing is adding unwanted complexity. Just try to keep things short and readable. the PEP8 guidelines is something you should read. It will provide some good references.\$\endgroup\$

  Mike - SMT

  – Mike - SMT

  2019-05-08 19:43:42 +00:00

  CommentedMay 8, 2019 at 19:43
• 1
  \$\begingroup\$@BrunoLemos I would. By changing the code you end up changing what the review would look like.\$\endgroup\$

  Mike - SMT

  – Mike - SMT

  2019-05-30 19:03:43 +00:00

  CommentedMay 30, 2019 at 19:03

Welcome to CodeReview! And welcome to coding! Publishing and reviewing your code isone of the best ways to get better at coding. And we're going to make you better, no matter how much it hurts. ;-)

First, congratulations! You've written a fair amount of code in a single project, and you managed to produce a somewhat complex app, with graphics, alternative input, event handling, etc. This is a pretty ambitious first project.

I have some suggestions about the organization and structure, and the coding style.

Organization and Structure

Modules

You have too many modules. A good starting rule for breaking code into different modules is this: always put everything in one file. By the time you need to break thatrule, you'll know what and how and when to break it. For now, you don't need to break it -- just put everything intocalculadora.py.

On a side note, the fact that you were importing a module at thebottom of one ofyour files instead of at the top is a sign that you should merge the modules together if possible. Needing to do that kind of thing should set off your internal alarmsthat something is wrong.

Functions

There are three good reasons to create a function: (1) to standardize operations that you perform more than one time; (2) to "abstract away" low-level operations to a separate layer; (3) to isolate a valuable operation for re-use.

Reason #3 is generally rare. But you aren't doing enough of #1 and #2. Consider this:

root = tk.Tk()root.geometry("640x640")visor = frame_display.DisplayContainer(root)numeros = frame_botoes.ButtonsContainer(root)root.mainloop()

The first four lines of that block "create the application". The fifth line "runs the application". You might put that in a class, if you have learned classes yet. Otherwise, just put that into two functions:

app = create_application()run_application(app)

Or consider this code:

self.button_1 = tk.Button(self, text="1", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(1))self.button_2 = tk.Button(self, text="2", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(2))self.button_3 = tk.Button(self, text="3", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(3))self.button_4 = tk.Button(self, text="4", padx=button_padx, pady=button_pady, command=lambda: calculadora.pressNumber(4))

There are more lines of this (5..0), but these four are enough to make the point: this is a repeated operation and could be a function!

What's more, these lines appear lower down:

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

These lines are "in parallel" with the button creation lines above. So they could be part of the same method. Let's try:

def make_button(self, text, row, column):    new_button = tk.Button(self, text=text, padx=self.BUTTON_PADX, pady=self.BUTTON_PADY,                           command=lambda: press_button(text))    new_button.grid(row=row, column=column, sticky=self.BUTTON_STICKY)    self.buttons.append(new_button)

Then you could replace a lot of that text with something like:

self.make_button('1', 0, 0)self.make_button('2', 0, 1)self.make_button('3', 0, 2)self.make_button('4', 1, 0)self.make_button('5', 1, 1)

Pro-tip: Visual Organization

When you're writing code, it's important to communicate to thenext guywhat you are trying to do. Sometimes the next guy is "future you" who will be reading this a year from now. Sometimes the next guy is another junior developer that will take over your project when you get promoted. But there's almost always going to be a "next guy" and your code is really written for him or her, more than for the compiler.

One trick you can use is to visually organize things. Then you will write code that "decodes" the visual organization. It's worth spending 15 minutes to make life easy for yourself, or for the next guy. Things like putting configuration into a docstring and parsing the string instead of putting 10 different values in separate quotation marks.

You could do something like this:

button_layout = """    1 2 3 +    4 5 6 -    7 8 9 *    ( 0 ) /    CCC . =""".strip('\n').splitlines()for row, line in enumerate(button_layout):    extra_col = 0    for col, ch in enumerate(line.split()):        if ch == 'CCC':            self.make_clear_button(row, col)            extra_col = 1        else:            self.make_button(ch, row, col + extra_col)self.num_rows = row + 1self.num_cols = col + 1

This would let you visually arrange the keys in different shapes, and the code would "figure out" where to put the buttons, and how many rows and columns were present.

Note that doing this providesabsolutely no value to your program. The buttons are going to be created no matter what. But it lets you explore different shapes for the window just by moving characters around, and it lets the "next guy" see and understand how the buttons are arranged in a way that 30+ lines ofrow=3, col=0 ... row=4, col=2 just cannot do.

Coding Style

PEP-8

The official Python coding style document isPEP8. You may have learned a different style from reading code written in Java or some other language. But you're going to be considered "out of spec" if you deviate from PEP-8.

That said, PEP-8 contains a lot of good advice for beginning coders. It's a fairly well-reasoned document, with just a few things that are totally wrong (IMO). But Iignore those things in favor of having a common standard, and you should too. Conform!

To quickly summarize:

• Usesnake_case for all names except classes. Classes arePascalCase just like every other language.

• UseALL_CAPS for "constants". If a class or object has an all-caps attribute,then it's a class constant or an object constant. If a module has an all-caps variable at the top, it's a module constant. This despite the fact ofmath.pi andmath.e andmath.tau. "Do as we say, not as we do." :-)

Importing names

You can import names from a module usingfrom module import name. Or you can import a module and refer tomodule.name instead. You should choose the style based onclarity and frequency of use.

For some reason, you do this:

from tkinter import Framefrom tkinter import StringVar

Then you make use ofFrame andStringVar 4 + 1 times, respectively. On the other hand, youdon't importButton but refer totk.Button 25 times!

I suggest that your default should be to not import any names explicitly, and to prefer to spell out themodule.name form. It's okay to abbreviate the module name, which you do (tkinter ->tk):

import tkinter as tkclass DisplayContainer(tk.Frame):    def __init__(...):        ...        self.text_display = tk.StringVar()

Then if you find yourself repeatingtk.Button 25 times (which you should not: seethe note about functions above) you can do an additional import of that one name. Or you could just stash it in a local variable if every occurrence is within the samefunction:

button = tk.Buttonthis = button(...)that = button(...)another = button(...)

Comments

If your comment says in English (or Portuguese!) the same thing the code says in Python, delete the comment. Don't do this:

# Call ButtonsContainer widgets creationself.createWidgets()

Comments should explain:

• Details that come from the problem domain

  # Per tax code, deduction does not apply if children > 12 yrs old

• Code constructs that are particularly dense or complex (particularly: nested comprehensions in Python)

  # Flatten list-of-lists, ignoring short names.all_planets = [planet for sublist in planets for planet in sublist if len(planet) < 6]

• Things that are not obvious from the code, including side effects.

  # Note: this call flushes stdout

• Things that are missing.

  # Note: NOT calling end_row() here!

• \$\begingroup\$thanks!!! I´ll refactor my code best as possible. Some tips are still a bit hard for me, as the button layout block you suggested.\$\endgroup\$

  Bruno Lemos

  – Bruno Lemos

  2019-05-08 14:41:58 +00:00

  CommentedMay 8, 2019 at 14:41
• 1
  \$\begingroup\$@BrunoLemos I would say no, because separatingcreate_widgets andarrange_widgets doesn't provide much value. Do youneed to have creation and arrangement separate? I don't thing so. But doing that means you have two functions, each of which probably has 25 lines, since I think there are 25 buttons? That means 50 lines of code and the information about buttons is in two different locations (button labels and callbacks are in one function, button positioning is in a different function).\$\endgroup\$

  aghast

  – aghast

  2019-05-08 22:50:55 +00:00

  CommentedMay 8, 2019 at 22:50
• 2
  \$\begingroup\$@BrunoLemos (cont'd) I think it makes more sense to have one function that does "everything for a button", and call that function 25 times. Then you have 30-ish lines of code (5 lines of function plus 25 calls) and all the information about a given button is in one place - the call to the function where the parameters are all in line.\$\endgroup\$

  aghast

  – aghast

  2019-05-08 22:52:52 +00:00

  CommentedMay 8, 2019 at 22:52
• 2
  \$\begingroup\$@BrunoLemos Suppose you have a function that opens a file, processes the contents, and closes the file. You could callopen() directly, or you could "abstract away" the low-level open call and write a function namedread_data() that would return the data. Sometimes it makes sense to have a "simple" function likeread_data so that you can write code likeread_data(); process_data(); write_data() instead of mixing the abstraction levels likef = open(); data = read(f); process_data(data); write_data()\$\endgroup\$

  aghast

  – aghast

  2019-05-24 21:26:50 +00:00

  CommentedMay 24, 2019 at 21:26
• 1
  \$\begingroup\$self.num_rows (&cols) are counts, which are 1-based. So they are 1 higher than the highest value returned byenumerate, which is 0-based. Thus, if youenumerate(['a', 'b']) you will get(0, 'a') and(1, 'b'), but thenum_items should betwo rather thanone. So I added one outside the loop.\$\endgroup\$

  aghast

  – aghast

  2019-05-24 21:30:44 +00:00

  CommentedMay 24, 2019 at 21:30

• python
• beginner
• python-3.x
• calculator
• tkinter