GUI interface for company specific calculator

Question

I created a customised GUI application in Python, which is a calculator that does some basic functions for my company. I would like to ask you for overall suggestions for code improvement.

# -*- coding: utf-8 -*-
"""
Created on Tue Mar 20 17:18:39 2018

@author: Dimitar Dimov
"""

import PIL
import Tkinter
import Tkinter as Tk
from math import *
from PIL import ImageTk, Image


class pmvCalculator(Tk.Frame):

    def __init__(self, parent, *args):
        Tk.Frame.__init__(self, parent, *args)
        self.parent = parent
        self.main = Tk.Frame(parent)
        self.main.grid(row = 0, column = 0)      
        self.initialise()

    def initialise(self):
        self.v = 0
        self.v2 = 0
        self.v3 = 0
        self.v4 = 0
        self.v5 = 0
        self.v6 = 0 
        self.v7 = 0
        self.v8 = 0
        self.houseValue = 40
        self.fivestoreyBuildingValue = 50
        self.tenstoreyBuildingValue = 60
        self.fifteenstoreyBuildingValue = 70

        self.grid()
        self.insertLogo()

        self.dropdownMenus()

        Tk.Button(self, text = "Show Options", command = self.buttonTwoPress).grid(row = 13, column = 2, pady =10)

        self.pmvcalcPicture()

    def insertLogo(self):
        self.logoFrame = Tk.Frame(self)
        self.path = PIL.Image.open("cast.gif.png")
        self.img = PIL.ImageTk.PhotoImage(self.path)
        Tk.Label(self, image = self.img).grid(row=1, column = 2, columnspan=4, rowspan=3, padx = 50, pady = 15)

    def dropdownMenus(self):
        self.dropdownFrame = Tk.Frame(self)
        choicesBuilding = {'House', '5 Storey Building', '10 Storey Building', '15 Storey Building'}
        choicesConstr = {'Traditional', '2-D', '3-D'}

        self.tkvar1 = Tk.StringVar(self)
        self.tkvar2 = Tk.StringVar(self)

        Tk.Label(self, text = "Enter your preferred type of building: ").grid(row=9, column = 2, sticky = Tk.W )
        Tk.OptionMenu(self, self.tkvar1, *choicesBuilding).grid(row=9, column = 3)

        Tk.Label(self, text = "Enter your preferred type of procurement: ").grid(row=11, column = 2, sticky = Tk.W)
        Tk.OptionMenu(self, self.tkvar2, *choicesConstr).grid(row=11, column = 3)

        return self.tkvar1, self.tkvar2

    def buttonTwoPress(self):        
        self.choice2()
        Tk.Button(self, text = "Calculate", command = self.fivestoreyBuilding).grid(row = 33, column = 2, pady =10, sticky = Tk.E)

    def choice2(self):    

        self.pmvar1 = self.tickBox("P-M Foundations", 15)
        self.pmvar2 = self.tickBox("P-M Frame Elements", 16)
        self.pmvar5 = self.tickBox("P-M Roof Solutions", 17)
        self.pmvar6 = self.tickBox("P-M Internal Walls", 18)
        self.pmvar7 = self.tickBox("P-M Doorsets", 19)
        self.pmvar8 = self.tickBox("P-M Bathrooms/ Kitchens", 20)
        self.pmvar9 = self.tickBox("P-M Wiring Solutions", 21)

        if self.tkvar1.get() == "5 Storey Building":
            self.pmvar10 = self.tickBox("P-M Plant Rooms", 22)
        else:
            self.tickBox("                                                         ",22)

        if self.tkvar1.get() == "5 Storey Building":
            self.pmvar11 = self.tickBox("P-M Risers", 23)
        else:
             self.tickBox("                                            ",23)

        if self.tkvar1.get() == "5 Storey Building":
            self.pmvar12 = self.tickBox("P-M HIU's", 24)
        else:
            self.tickBox("                                          ",24)

        if self.tkvar1.get() == "5 Storey Building":
            self.pmvar4 = self.tickBox("P-M SIPS Facades", 25)
        else:
            self.tickBox("                                                        ",25)

        if self.tkvar1.get() == "5 Storey Building":
            self.pmvar3 = self.tickBox("Large Format Facade Materials", 26)
        else:
            self.pmvar3 = self.tickBox("                                                     ",26)

        if self.tkvar2.get() == "2-D":
            self.pmvar13 = self.tickBox("P-M Structural Systems inclduing Insulation:", 27)
        else: 
           self.pmvar13 = self.tickBox("                                                                          ", 27)

        if self.tkvar2.get() == "2-D":
            self.pmvar14 = self.tickBox("P-M Facade Solution", 28)
        else:
            self.pmvar14 = self.tickBox("                                                     ", 28)

        if self.tkvar2.get() == "3-D":
            self.pmvar15 = self.tickBox("P-M Core", 29)
        else:
            self.pmvar15 = self.tickBox("                                                     ", 29)    

        if self.tkvar2.get() == "3-D":
            self.pmvar16 = self.tickBox("P-M Integrated Modular Facade", 30)
        else:
            self.pmvar16 = self.tickBox("                                                       ", 30)

        if self.tkvar2.get() == "3-D":
            self.pmvar17 = self.tickBox("P-M Common Areas (Unfurnished)", 31)
        else:
            self.pmvar17 = self.tickBox("                                                             ", 31)    

        if self.tkvar2.get() == "3-D":
            self.pmvar18 = self.tickBox("P-M Common Areas (Furnished)", 32)
        else:
            self.pmvar18 = self.tickBox("                                                         ", 32)    


    def fivestoreyBuilding(self):

        if self.pmvar1.get() == True:
            self.v = 0.5
        else:
            self.v = 0
        if self.pmvar2.get() == True:
            self.v2 = 3
        else:
            self.v2 = 0

        if self.pmvar3.get() == True:
            self.v3 = 0.5
        else:
            self.v3 = 0

        if (self.tkvar1.get() == "5 Storey Building" and self.pmvar4.get() == True):
            self.v4 = 3
        else:
            self.v4 = 0

        if self.pmvar5.get() == True:
            self.v5 = 1
        else:
            self.v5 = 0
        if self.pmvar6.get() == True:
            self.v6 = 1
        else:
            self.v6 = 0
        if self.pmvar7.get() == True:
            self.v7 = 1
        else:
            self.v7 = 0
        if self.pmvar8.get() == True:
            self.v8 = 2
        else:
            self.v8 = 0
        if self.pmvar9.get() == True:
            self.v9 = 0.5
        else:
            self.v9 = 0
        if (self.tkvar1.get() == "5 Storey Building" and self.pmvar10.get() == True):
            self.v10 = 1
        else:
            self.v10 = 0

        if (self.tkvar1.get() == "5 Storey Building" and self.pmvar11.get() == True):
            self.v11 = 1
        else:
            self.v11 = 0

        if ( self.tkvar1.get() == "5 Storey Building" and self.pmvar12.get() == True):
            self.v12 = 1
        else:
            self.v12 = 0

        if (self.tkvar2.get() == "2-D" and self.pmvar13.get() == True):
            self.v13 = 0.5
        else:
            self.v13 = 0

        if (self.tkvar2.get() == "2-D" and self.pmvar14.get() == True):
            self.v14 = 1.5
        else:
            self.v14 = 0

        if (self.tkvar2.get() == "3-D" and self.pmvar15.get() == True):
            self.v15 = 0.5
        else:
            self.v15 = 0

        if (self.tkvar2.get() == "3-D" and self.pmvar16.get() == True):
            self.v16 = 5
        else:
            self.v16 = 0

        if (self.tkvar2.get() == "3-D" and self.pmvar17.get() == True):
            self.v17 = 7
        else:
            self.v17 = 0

        if (self.tkvar2.get() == "3-D" and self.pmvar18.get() == True):
            self.v18 = 10
        else:
            self.v18 = 0

        if (self.tkvar1.get() == "House"):
            self.houseValue = 40
        else:
            self.houseValue = 0

        if (self.tkvar1.get() == "5 Storey Building"):
            self.fivestoreyBuildingValue = 50
        else:
            self.fivestoreyBuildingValue = 0

        finalPMV2 = Tk.IntVar()  
        finalPMV2 = (self.houseValue + self.fivestoreyBuildingValue
                     + self.v + self.v2 + self.v3 + self.v4 + self.v5
                     + self.v6 + self.v7 + self.v8 + self.v9 + self.v10 + self.v11 + self.v12
                     + self.v13+ self.v14+ self.v15+ self.v16+ self.v17 + self.v18)
        Tk.Label(self, text = '{} %'.format(finalPMV2)).grid(row = 33, column =3, pady=10)
        return finalPMV2

    def pmvcalcPicture(self):

        self.path2 = PIL.Image.open("pmv_calc.png")
        self.path2 = self.path2.resize((600,300), Image.ANTIALIAS) 
        self.path2.save('update_pm_image.ppm', 'ppm')
        self.img2 = PIL.ImageTk.PhotoImage(self.path2) 
        Tk.Label(self, image = self.img2).grid(row=14, column = 6, columnspan=3, rowspan=30, padx = 20, pady = 15)

    def tickBox(self, label, newrow):
        var = Tk.IntVar()
        Tk.Checkbutton(self, text = label, variable = var).grid(row = newrow, column = 2,sticky = Tk.W)
        return var

if __name__ == '__main__':
    root = Tk.Tk()
    root.title("Cast PMV Calculator")
    root.geometry("1100x800")

    app = pmvCalculator(root)

    root.mainloop()

Answer 1

Firstly you could simplify some if-statements by doing if self.pmvar1.get(): instead of if self.pmvar1.get() == True: since the if will verify whether the condition is True by itself.

Furthermore, if I didn't miss something it's possible to merge some if statements within choice2 into one because if self.tkvar1.get() == "5 Storey Building": seems to be repeated quite often. But, as I said, I'm really tired right now and maybe there is a reason I just didn't see.

Answer 2

naming

self.v8 is very unclear in what it does. The same goes for buttonTwoPress and some of the other function names. The names of the functions and variables are part of the documentation, so use clear names. Also, try to follow Pep-8.

parametrize

You have 3 building types and some 20 options. Instead of hard-coding the name and cost of each of those in the GUI code, it is better to keep those settings somewhere else in a configuration

BuildingOption = namedtuple('BuildingOption', ['name', 'cost'])
BUILDING_TYPES = OrderedDict([
    ('house', {
        'cost': 40
    }),
    ('5 Storey Building', {
        'cost': 50,
        'options': [
            BuildingOption('P-M Plant Rooms', 1),
            BuildingOption('P-M Risers', 1),
            BuildingOption("P-M HIU's", 1),
            BuildingOption('P-M SIPS Facades', 1),
            BuildingOption('Large Format Facade Materials', 1),
        ],
    }),
    ('10 Storey Building', {
        'cost': 60,
    }),
    ('15 Storey Building', {
        'cost': 60,
    }),
])

GENERAL_CHOICES = [
    BuildingOption('P-M Foundations', 1),
    BuildingOption('P-M Frame Elements', 1),
    BuildingOption('P-M Roof Solutions', 1),
    BuildingOption('P-M Internal Walls', 1),
    BuildingOption('P-M Doorsets', 1),
    BuildingOption('P-M Bathrooms/ Kitchens', 1),
    BuildingOption('P-M Wiring Solutions', 1),
]
CONSTRUCTION_CHOICES = {
    'Traditional': [],
    '2-D': [
        BuildingOption('P-M Structural Systems including Insulation:', 1),
        BuildingOption('P-M Facade Solution', 1),
    ],
    '3-D': [
        BuildingOption('P-M Core', 1),
        BuildingOption('P-M Integrated Modular Facade', 1),
        BuildingOption('P-M Common Areas (Unfurnished)', 1),
        BuildingOption('P-M Common Areas (Furnished)', 1),
    ],
}

this way you can keep the thickboxes in a dict self.tickboxes option as the key. The options can be accessed with these helper functions

def get_building_options(building_type):
    return BUILDING_TYPES[building_type.get()].get('options', [])


def get_construction_options(construction_choice):
    return CONSTRUCTION_CHOICES[construction_choice.get()]

populate dropdown

I renamed dropdownMenus tot populate_dropdown, and used the keys of the building types and construction choices as items. I gave tkvar1 and tkvar2 also more clear names.

def populate_dropdown(self):
    self.dropdownFrame = Tk.Frame(self)

    building_type = Tk.StringVar(self)
    construction_choice = Tk.StringVar(self)

    Tk.Label(self, text='Enter your preferred type of building: ').grid(row=9, column=2, sticky=Tk.W)
    Tk.OptionMenu(self, building_type, *BUILDING_TYPES.keys()).grid(row=9, column=3)

    Tk.Label(self, text='Enter your preferred type of procurement: ').grid(row=11, column=2, sticky=Tk.W)
    Tk.OptionMenu(self, construction_choice, *CONSTRUCTION_CHOICES.keys()).grid(row=11, column=3)
    return building_type, construction_choice

populating tickboxes

I renamed choice2 to populate_tickboxes, and it can be as simple as:

def populate_tickboxes(self):
    self.clear_tickboxes()
    tickboxes = (
        GENERAL_CHOICES
        + get_building_options(self.building_type)
        + get_construction_options(self.construction_choice)
    )
    for row, option in enumerate(tickboxes, 15):
        self.tickboxes[option] = self.make_tickbox(option.name, row)
def show_options(self):
    self.populate_tickboxes()
    Tk.Button(self, text='Calculate', command=self.calculate_cost).grid(row=33, column=2, pady=10, sticky=Tk.E)

If you want to group the tickboxes into the different types, that should be quite easy too

clear the tickboxes

To clear the tickboxes, you also need a reference to the tickbox, and not only to the var, so I changed self.tickBox to:

TickBoxTuple = namedtuple('TickBox', ['var', 'tickbox'])
def make_tickbox(self, label, newrow):
    var = Tk.IntVar()
    tickbox = Tk.Checkbutton(self, text=label, variable=var)
    tickbox.grid(row=newrow, column=2, sticky=Tk.W)
    return TickBoxTuple(var, tickbox)

Then clearing the tickboxes becomes as simple as:

def clear_tickboxes(self):
    while self.tickboxes:
        _, tickbox = self.tickboxes.popitem()
        tickbox.tickbox.destroy()

calculate cost

I renamed fivestoreyBuilding to calculate_cost. The function to calculate the costs is then this very concise.

def calculate_cost(self):
    total_cost = sum(tickbox.var.get() * option.cost for (option, tickbox) in self.tickboxes.items())
    total_cost += BUILDING_TYPES[self.building_type.get()]['cost']
    Tk.Label(self, text='{} %'.format(total_cost)).grid(row=33, column=3, pady=10)
    return total_cost

init

I try to declare all instance variables in the __init__. That way it is clearest

def __init__(self, parent, *args):
    Tk.Frame.__init__(self, parent, *args)
    self.parent = parent
    self.main = Tk.Frame(parent)
    self.main.grid(row=0, column=0)
    self.tickboxes = {}

    self.grid()
    self.logo = self.insert_logo()
    self.logo.grid(row=1, column=2, columnspan=4, rowspan=3, padx=50, pady=15)

    self.option_button = Tk.Button(self, text='Show Options', command=self.show_options)
    self.option_button.grid(row=13, column=2, pady=10)

    self.dropdown_frame, self.building_type, self.construction_choice = self.populate_dropdown()

    self.pmv_calc_picture = self.add_pmv_calc_picture()
    self.pmv_calc_picture.grid(row=14, column=6, columnspan=3, rowspan=30, padx=20, pady=15)

def insert_logo(self):
    logoFrame = Tk.Frame(self)  # is this used?
    path = PIL.Image.open("cast.gif.png")
    img = PIL.ImageTk.PhotoImage(path)
    return Tk.Label(self, image=img)

def add_pmv_calc_picture(self):

    path = PIL.Image.open("pmv_calc.png")
    path = path.resize((600, 300), Image.ANTIALIAS)
    path.save('update_pm_image.ppm', 'ppm')
    img = PIL.ImageTk.PhotoImage(path)
    return Tk.Label(self, image=img)

summary

All in all, in this code, way too much is hardcoded. Learn to use the correct data structure. The things I changed are just a few suggestions. You can seperate the presentation and logic a lot further, perhaps by the inclusion of a few Classes

My complete attempt can be found here