GUI interface for company specific calculator

The name of the pictureThe name of the pictureThe name of the pictureClash Royale CLAN TAG#URR8PPP





.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;







up vote
3
down vote

favorite












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






share|improve this question

















  • 1




    I would like to receive a review on the computation part if possible. That would help me structure my thoughts and become a better programmer. thank you for your time and attention! I appreciate it very much.
    – Dimitar Dimov
    Mar 22 at 15:26











  • I added the python-2.7 tag based off your imports, feel free to correct it if I’m wrong.
    – Mathias Ettinger
    Mar 22 at 15:45










  • yes correct - this is python 2.7, written in Spyder IDE
    – Dimitar Dimov
    Mar 22 at 15:52










  • Please share links to those images you used
    – Billal BEGUERADJ
    Apr 4 at 7:39










  • Just hide the image if you try to run the code, its not really important. The mechanics of the calculator are what and code design are what I'm after
    – Dimitar Dimov
    Apr 5 at 9:21
















up vote
3
down vote

favorite












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






share|improve this question

















  • 1




    I would like to receive a review on the computation part if possible. That would help me structure my thoughts and become a better programmer. thank you for your time and attention! I appreciate it very much.
    – Dimitar Dimov
    Mar 22 at 15:26











  • I added the python-2.7 tag based off your imports, feel free to correct it if I’m wrong.
    – Mathias Ettinger
    Mar 22 at 15:45










  • yes correct - this is python 2.7, written in Spyder IDE
    – Dimitar Dimov
    Mar 22 at 15:52










  • Please share links to those images you used
    – Billal BEGUERADJ
    Apr 4 at 7:39










  • Just hide the image if you try to run the code, its not really important. The mechanics of the calculator are what and code design are what I'm after
    – Dimitar Dimov
    Apr 5 at 9:21












up vote
3
down vote

favorite









up vote
3
down vote

favorite











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






share|improve this 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()








share|improve this question












share|improve this question




share|improve this question








edited Apr 3 at 2:37









Jamal♦

30.1k11114225




30.1k11114225









asked Mar 22 at 15:06









Dimitar Dimov

162




162







  • 1




    I would like to receive a review on the computation part if possible. That would help me structure my thoughts and become a better programmer. thank you for your time and attention! I appreciate it very much.
    – Dimitar Dimov
    Mar 22 at 15:26











  • I added the python-2.7 tag based off your imports, feel free to correct it if I’m wrong.
    – Mathias Ettinger
    Mar 22 at 15:45










  • yes correct - this is python 2.7, written in Spyder IDE
    – Dimitar Dimov
    Mar 22 at 15:52










  • Please share links to those images you used
    – Billal BEGUERADJ
    Apr 4 at 7:39










  • Just hide the image if you try to run the code, its not really important. The mechanics of the calculator are what and code design are what I'm after
    – Dimitar Dimov
    Apr 5 at 9:21












  • 1




    I would like to receive a review on the computation part if possible. That would help me structure my thoughts and become a better programmer. thank you for your time and attention! I appreciate it very much.
    – Dimitar Dimov
    Mar 22 at 15:26











  • I added the python-2.7 tag based off your imports, feel free to correct it if I’m wrong.
    – Mathias Ettinger
    Mar 22 at 15:45










  • yes correct - this is python 2.7, written in Spyder IDE
    – Dimitar Dimov
    Mar 22 at 15:52










  • Please share links to those images you used
    – Billal BEGUERADJ
    Apr 4 at 7:39










  • Just hide the image if you try to run the code, its not really important. The mechanics of the calculator are what and code design are what I'm after
    – Dimitar Dimov
    Apr 5 at 9:21







1




1




I would like to receive a review on the computation part if possible. That would help me structure my thoughts and become a better programmer. thank you for your time and attention! I appreciate it very much.
– Dimitar Dimov
Mar 22 at 15:26





I would like to receive a review on the computation part if possible. That would help me structure my thoughts and become a better programmer. thank you for your time and attention! I appreciate it very much.
– Dimitar Dimov
Mar 22 at 15:26













I added the python-2.7 tag based off your imports, feel free to correct it if I’m wrong.
– Mathias Ettinger
Mar 22 at 15:45




I added the python-2.7 tag based off your imports, feel free to correct it if I’m wrong.
– Mathias Ettinger
Mar 22 at 15:45












yes correct - this is python 2.7, written in Spyder IDE
– Dimitar Dimov
Mar 22 at 15:52




yes correct - this is python 2.7, written in Spyder IDE
– Dimitar Dimov
Mar 22 at 15:52












Please share links to those images you used
– Billal BEGUERADJ
Apr 4 at 7:39




Please share links to those images you used
– Billal BEGUERADJ
Apr 4 at 7:39












Just hide the image if you try to run the code, its not really important. The mechanics of the calculator are what and code design are what I'm after
– Dimitar Dimov
Apr 5 at 9:21




Just hide the image if you try to run the code, its not really important. The mechanics of the calculator are what and code design are what I'm after
– Dimitar Dimov
Apr 5 at 9:21










2 Answers
2






active

oldest

votes

















up vote
1
down vote













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.






share|improve this answer























  • The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
    – Dimitar Dimov
    Apr 4 at 7:09

















up vote
0
down vote













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






share|improve this answer





















    Your Answer




    StackExchange.ifUsing("editor", function ()
    return StackExchange.using("mathjaxEditing", function ()
    StackExchange.MarkdownEditor.creationCallbacks.add(function (editor, postfix)
    StackExchange.mathjaxEditing.prepareWmdForMathJax(editor, postfix, [["\$", "\$"]]);
    );
    );
    , "mathjax-editing");

    StackExchange.ifUsing("editor", function ()
    StackExchange.using("externalEditor", function ()
    StackExchange.using("snippets", function ()
    StackExchange.snippets.init();
    );
    );
    , "code-snippets");

    StackExchange.ready(function()
    var channelOptions =
    tags: "".split(" "),
    id: "196"
    ;
    initTagRenderer("".split(" "), "".split(" "), channelOptions);

    StackExchange.using("externalEditor", function()
    // Have to fire editor after snippets, if snippets enabled
    if (StackExchange.settings.snippets.snippetsEnabled)
    StackExchange.using("snippets", function()
    createEditor();
    );

    else
    createEditor();

    );

    function createEditor()
    StackExchange.prepareEditor(
    heartbeatType: 'answer',
    convertImagesToLinks: false,
    noModals: false,
    showLowRepImageUploadWarning: true,
    reputationToPostImages: null,
    bindNavPrevention: true,
    postfix: "",
    onDemand: true,
    discardSelector: ".discard-answer"
    ,immediatelyShowMarkdownHelp:true
    );



    );








     

    draft saved


    draft discarded


















    StackExchange.ready(
    function ()
    StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190209%2fgui-interface-for-company-specific-calculator%23new-answer', 'question_page');

    );

    Post as a guest






























    2 Answers
    2






    active

    oldest

    votes








    2 Answers
    2






    active

    oldest

    votes









    active

    oldest

    votes






    active

    oldest

    votes








    up vote
    1
    down vote













    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.






    share|improve this answer























    • The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
      – Dimitar Dimov
      Apr 4 at 7:09














    up vote
    1
    down vote













    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.






    share|improve this answer























    • The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
      – Dimitar Dimov
      Apr 4 at 7:09












    up vote
    1
    down vote










    up vote
    1
    down vote









    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.






    share|improve this answer















    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.







    share|improve this answer















    share|improve this answer



    share|improve this answer








    edited Apr 3 at 7:20









    Sam Onela

    5,82961544




    5,82961544











    answered Apr 3 at 2:35









    Robin Weiland

    112




    112











    • The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
      – Dimitar Dimov
      Apr 4 at 7:09
















    • The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
      – Dimitar Dimov
      Apr 4 at 7:09















    The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
    – Dimitar Dimov
    Apr 4 at 7:09




    The reason why I repeat them is because upon choosing a different option from the dropdown menu and then pressing the button "Show Options", some of the checkboxes appear or disappear. I was thinking of smarter way to do this - perhaps linking a new frame to different options in the dropdown ("House", "5 Storey" etc) and invoking a function to show/hide the frame, however, I didn't know how to code it.
    – Dimitar Dimov
    Apr 4 at 7:09












    up vote
    0
    down vote













    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






    share|improve this answer

























      up vote
      0
      down vote













      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






      share|improve this answer























        up vote
        0
        down vote










        up vote
        0
        down vote









        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






        share|improve this answer













        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







        share|improve this answer













        share|improve this answer



        share|improve this answer











        answered May 4 at 13:28









        Maarten Fabré

        3,204214




        3,204214






















             

            draft saved


            draft discarded


























             


            draft saved


            draft discarded














            StackExchange.ready(
            function ()
            StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f190209%2fgui-interface-for-company-specific-calculator%23new-answer', 'question_page');

            );

            Post as a guest













































































            Popular posts from this blog

            Greedy Best First Search implementation in Rust

            Function to Return a JSON Like Objects Using VBA Collections and Arrays

            C++11 CLH Lock Implementation