2
\$\begingroup\$

It is supposed to take an arbitrary number of arguments, turn those arguments into buttons, and arrange those buttons in a way to give each individual button the largest area possible. Is there a better way to do this? Be as thorough as you like.

import sys
import tkinter as tk
import os

from tkinter import *
###functions
def calcRowsAndColumns(area = None): 
    if(area==None):
        area = len(sys.argv) -1
    if(area==1):
        return (1,1)
    rectanglePropertiesList = []
    for rows in range(1, area):
        if(area % rows == 0):
            columns = int(area/rows)
            rectanglePropertiesList.append([rows,columns]) 
    if (len(rectanglePropertiesList) == 1): #This deals with prime numbers
        return calcRowsAndColumns(area+1)       
    return tuple(smallestPerimeter(rectanglePropertiesList))                

def smallestPerimeter(propertyList):
    return propertyList[propertyList.index(min(propertyList, key=sum))] 

def preserve(gFile):
    print(gFile)
    argList.remove(gFile)
    for i in argList:
        os.remove(i)

def relwidth(root):
    return root.winfo_screenwidth()//(len(argList))-20
    #Subtracting 20 to account for padding

def relheight(root):
    return root.winfo_height()//(len(argList))-20
###Code to be executed
rowsAndColumnTuple = calcRowsAndColumns(None)
root = tk.Tk()
root.attributes("-fullscreen", True)
root.config(bg="white")
root.grid()

listButton = []

argList = [i for i in sys.argv[1:]]

for i in argList:
    listButton.append(Button(root, text=i.rpartition(os.sep)[2],
                             font="-size 20", command=lambda x=i: preserve(x), 
                             #lambda used here to allow parameters to be passed to the preserve function
                             padx=10, pady=10,
                             bg ="black", fg = "red"))

iterButtons = iter(listButton)

for i in range(rowsAndColumnTuple[0]):
    root.grid_rowconfigure(i, weight=1)
    for j in range(rowsAndColumnTuple[1]):
        root.grid_columnconfigure(j, weight=1)
        try:
            button=next(iterButtons)
        except StopIteration:
            print("reached end of list : ignoring remaining grid cells")
            break   
        button.grid(row=i,column=j)

root.update()

for i in listButton:
    i.config(height=relheight(root), width=relwidth(root))
    #This resizes the buttons to the correct size

root.mainloop()
\$\endgroup\$

1 Answer 1

5
\$\begingroup\$

Don't import tkinter twice

You're doing this:

import tkinter as tk
from tkinter import *

Remove the second import since you are already importing tkinter. That means that everywhere you reference a tkinter class or constant you need to prefix it with tk. (eg: tk.Button). This is much preferable over a wildcard import.

Let tkinter compute the size of the widgets

Since you are creating a grid, there's no reason to explicitly configure the width and the heights of the buttons. Instead, use the sticky attribute to tell tkinter to grow (or shrink) the widget to fit the area given to it.

Because you are properly giving the rows and columns weight, all you need to do is add the sticky attribute when adding the widget to the grid, and then remove the code that explicitly configures the width and height.

For example:

button.grid(row=i,column=j, sticky="nsew")

Don't use variables i and j to represent rows and columns

If you are iterating over rows and columns, use variable names to make that more clear. Variables named i and j carry no meaning. Use row and column (or something similar -- r, c, row_number, column_number, etc) instead. Also, there's no reason to save the results of calcRowsAndColumns to a tuple. Save the individual values as scalar variables.

In other words, instead of this:

rowsAndColumnTuple = calcRowsAndColumns(None)
...
For i in range(rowsAndColumnTuple[0]):
    root.grid_rowconfigure(i, weight=1)
    for j in range(rowsAndColumnTuple[1]):
        root.grid_columnconfigure(j, weight=1)

Do this:

rows, columns = calcRowsAndColumns(None)
for row in range(rows):
    root.grid_rowconfigure(row, weight=1)
    for column in range(columns):
        root.grid_columnconfigure(column, weight=1)

Remove the call to update

With the above changes, you don't need to call root.update() toward the end of the script.

Use classes

Your code is hard to understand at a high level. Part of the reason for this is that you have to define some functions before you create the actual GUI. That means I have to scan through the lines of code looking for where you create the window.

While some people find the use of classes daunting, they make writing GUI code much easier. You should consider moving all of this code into a single class, which will allow you to write the code in a manner that makes it more readable.

A description of my recommendation is in this answer on stackoverflow: https://stackoverflow.com/a/17470842/7432. In a nutshell, it suggests starting with this template:

import tkinter as tk

class MainApplication(tk.Frame):
    def __init__(self, parent, *args, **kwargs):
        tk.Frame.__init__(self, parent, *args, **kwargs)
        self.parent = parent
        # <create the rest of your GUI here>

if __name__ == "__main__":
    root = tk.Tk()
    MainApplication(root).pack(fill="both", expand=True)
    root.mainloop()

You can then move all of your code into MainApplication . This allows the main logic of creating the GUI to be up front rather than buried deep in the code.

\$\endgroup\$
2
  • \$\begingroup\$ Thanks for your input! I will rewrite this as a class. Do you recommend moving the functions to the inside of the main class? Also, I tried using the sticky attribute, but it would not work for some reason. I'll look into the documentation, since I was probably using it incorrectly. \$\endgroup\$
    – Jal
    Dec 17, 2016 at 18:22
  • 1
    \$\begingroup\$ "Do you recommend moving the functions to the inside of the main class" - yes. There's no strict requirement to do so, but I personally think it makes the code easier to understand, assuming the functions will only be used by this class. Shared functions are normally outside the class, and class-specific functions are inside. \$\endgroup\$ Dec 17, 2016 at 18:33

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.