3
\$\begingroup\$

This program is meant to ease making mods for a game. A mod is made with XML. I used the lxml library to handle making the XML. I used PyQt for the GUI and this code is the functionality of the GUI. Most functions do just what their name says: open file opens a file, save saves, addFunction makes sure that button that says add function actually adds a function.

This code is long and so as much as help to improve one function would be awesome. It works, but I know this code can be improved I just need feedback. I am no expert yet, so anything would be awesome feedback.

It needs a GUI .py file to be run. It is long and takes too much space, but if it is a must then I guess I can include it anyways.

This program basically helps make XML files with certain tags for a game. The game uses XML files to add features created by the users. Making these XML files is extremely easy but tedious and so I decided to make this program to achieve just that. It works so far as it is expected, but it has became very large, and I fear I am making it more complicated than it has to be and that it could be less complex.

from lxml import etree
from PyQt4 import QtGui
import sys
import design
import dialog
import modBuilder as mb
import modtoolbar as mtb
import errorMessages as em
from PyQt4.QtCore import *
from PyQt4.QtGui import *
from os import path
from copy import deepcopy
import os
try:
 from collections import OrderedDict
except ImportError:
 OrderedDict = dict
import time
class ExampleApp(QtGui.QMainWindow, design.Ui_MainWindow):
 def __init__(self):
 super().__init__()
 QtGui.QApplication.setStyle(QtGui.QStyleFactory.create('plastique'))
 self.setupUi(self)
 self.openBttn.clicked.connect(lambda:self.openFileHandler())
 self.saveBttn.clicked.connect(lambda:self.save(self.tree))
 self.newMBttn.clicked.connect(lambda:self.createNewMod())
 self.confBttn.clicked.connect(lambda: self.setNewSoftwareTypeValues())
 self.setNameGeneratorBttn.clicked.connect(lambda: self.setNameGenerator(self.tree))
 self.deleteFeatureBttn.clicked.connect(lambda:self.deleteFeature(self.tree))
 self.editFeatureBttn.clicked.connect(lambda:self.populateFeatureFields(self.tree))
 self.savechanges.clicked.connect(lambda:self.saveChangesToFeature(self.tree))
 self.addDependencyBttn.clicked.connect(lambda:self.addOSDependency())
 self.addVisualDependencyBttn.clicked.connect(lambda:self.addVisDependency())
 self.addAudioDependencyBttn.clicked.connect(lambda:self.addAudDependency())
 self.addCustDepBttn.clicked.connect(lambda:self.addCustomDependency(str(self.softwareDepLE.text()),str(self.featureDepLE.text())))
 self.deleteDependencyBttn.clicked.connect(lambda:self.deleteDependency(self.tree))
 self.addFeatureBttn.clicked.connect(lambda:self.addFeature(self.tree))
 self.FORCEDCHECKBOX.stateChanged.connect(lambda:self.checkForcedCheckBoxStatus())
 self.FROMCHECKBOX.stateChanged.connect(lambda:self.checkFromCheckBoxStatus())
 self.VITALCHECKBOX.stateChanged.connect(lambda:self.checkVitalCheckBoxStatus())
 self.OSCHECKBOX.stateChanged.connect(lambda: self.dependencyCheckStatus())
 self.VISCHECKBOX.stateChanged.connect(lambda:self.dependencyCheckStatus())
 self.AUDCHECKBOX.stateChanged.connect(lambda:self.dependencyCheckStatus())
 self.tree = ""
 self.depco = 1
 self.depco2 = 1
 self.depco3 = 1
 self.cdepco = 1
 self.oslist = {}
 self.audlist = {}
 self.vislist = {}
 self.customDependencyList = {}
 self.listOfFeatures = {}
 self.keyIncidentDict = {}
 self.featureToEdit = "nothing"
 self.functionKey = 0
 self.vitalStatus = False
 self.fromStatus = False
 self.forcedStatus = False
 mainMenu = self.defineMenuBar()
 fileMenu = self.addMenuToMenuBar(mainMenu, '&File')
 self.defineCloseFunction(mainMenu,fileMenu)
 self.defSaveFunction(mainMenu,fileMenu)
 self.defineNewMod(mainMenu,fileMenu)
 self.defineSaveAsFunction(mainMenu,fileMenu)
 def defineMenuBar(self):
 mainMenu = self.menuBar() #
 return mainMenu #
 def addMenuToMenuBar(self,menu,name):
 fileMenu = menu.addMenu(name) #
 return fileMenu #
 def addActionsToMenuBar(self,fileMenu,action):
 fileMenu.addAction(action) #
 def defineCloseFunction(self,menuBar,fileMenu):
 closeAction = QtGui.QAction("&Close", self) #
 closeAction.setShortcut("Ctrl+Q") #
 closeAction.triggered.connect(self.close) #
 self.addActionsToMenuBar(fileMenu,closeAction) #
 def defineSaveAsFunction(self,menuBar,fileMenu):
 saveAsAction = QtGui.QAction("&Save As", self)
 saveAsAction.triggered.connect(self.saveAs)
 self.addActionsToMenuBar(fileMenu,saveAsAction)
 def defSaveFunction(self,menuBar,fileMenu):
 tree = self.tree # Stores self.tree in the variable tree
 saveAction = QtGui.QAction("&Save", self) # Creates QAction to Save and stores it in the variable saveAction
 saveAction.setShortcut("Ctrl+S") # Sets saveAction shortcut to Ctrl + S
 saveAction.triggered.connect(self.executeSave) # Connects saveAction to executeSave
 self.addActionsToMenuBar(fileMenu,saveAction) # Executes function addActionsToMenuBar
 def defineNewMod(self,menuBar,fileMenu): 
 newAction = QtGui.QAction("&New Mod", self) # Creates QAction, calls it New Mod and stores it in the varaible newAction
 newAction.setShortcut("Ctrl+N") # Sets newAction shortcut to Ctrl + N
 newAction.triggered.connect(self.createNewMod) # Connects newAction ('Ctrl + N') to function createNewMod
 self.addActionsToMenuBar(fileMenu,newAction) # Executes addActionsToMenuBar
 def close(self):
 choice = QtGui.QMessageBox.question(self, 'Warning',"You are about to exit. All unsaved progress will be lost. Do you want to continue?",QtGui.QMessageBox.Yes | QtGui.QMessageBox.No)
 if choice == QtGui.QMessageBox.Yes: # Checks if choice is equal to the user response of yes
 sys.exit() # Executes system function exit
 def executeSave(self):
 self.save(self.tree) # Executes the function save
 def openFileHandler(self):
 self.filename = QtGui.QFileDialog.getOpenFileName(self, 'Open File')
 self.directory = os.path.dirname(self.filename)
 self.openFile()
 def openFile(self): 
 try:
 with open(self.filename) as f: # Opens the filename as f 
 self.tree = etree.parse(f) # parses f and stores it in the variable self.tree
 self.statusBar().showMessage('File Loaded',1500) # Shows message 'File loaded' 
 self.addFeatureBttn.setEnabled(True) # Enables addFeatureBttn button
 self.funcName.setEnabled(True)
 self.PopulateSoftwareTypeFields(self.tree) # Executes the function PopulateSoftwareTypeFields
 self.addFeaturesToComboBox(self.tree) # Executes the function addFeaturesToComboBox
 except:
 if self.filename == '':
 pass
 else:
 em.showUnexpectedError(self)
 #def createFile(self):
 def save(self, tree):
 try:
 with open(self.filename, 'wb+') as f: # Opens filename as f with the intention to write
 tree.write(f, pretty_print=True) # Writes tree into filename
 self.statusBar().showMessage('Saved',1500) # Shows message 'Saved'
 except:
 if self.filename == '': # Checks if filename is equal to empty string
 em.showSaveError(self) # Shows saverError imported from errorMessages
 else: 
 em.showSaveErrorTwo(self) # Shows saverErrorTwo imported from errorMessages 
 def saveAs(self,tree):
 try:
 self.filename = QtGui.QFileDialog.getSaveFileName(self, 'Save As')
 self.directory = QtGui.QFileDialog.getExistingDirectory(self, 'Choose your directory') 
 filename = os.path.basename(os.path.normpath(self.filename))
 with open(self.filename,'wb') as f:
 tree.write(f, pretty_print=True)
 directory = open(self.directory,'w')
 directory.write(filename)
 directory.close()
 self.statusBar().showMessage('Saved',1500)
 except:
 pass 
 def createNewMod(self):
 try:
 self.filename = QtGui.QFileDialog.getSaveFileName(self, 'Choose a name for your mod')
 numberOfFeatures = self.numberOfFeatures.value() # Gets the value of self.numberOfFeatures and stores it in numberOfFeatures 
 mb.createMod(numberOfFeatures,self.filename) # Executes the createMod function imported from modBuilder
 self.settingModForChanges(self.filename) # Executes settingModForChanges function
 self.funcName.setEnabled(True) # Enables funcName
 self.addFeatureBttn.setEnabled(True) # Enables addFeatureBttn button
 directory = QtGui.QFileDialog.getExistingDirectory(self, 'Choose your directory')
 self.statusBar().showMessage('New Mod Created',1500) # Shows message 'New Mod Created'
 except: 
 em.errorCreatingMod(self) # Show error message for error while creating mod
 def settingModForChanges(self,modName):
 self.openFile()
 self.PopulateSoftwareTypeFields(self.tree) # Executes the function PopulateSoftwareTypeFields
 self.addFeaturesToComboBox(self.tree) # Executes the function addFeaturesToComboBox
 def PopulateSoftwareTypeFields(self,tree):
 Name = (tree.find('Name').text) # Finds the tag Name, gets its text value and stores it in the variable Name
 Description = (tree.find("Description").text) # Finds the tag Description, gets its text value and stores it in the variable Description
 Unlock =(tree.find("Unlock").text) # Finds the tag Unlock, gets its text value and stores it in the variable Unlock
 Population = (tree.find("Population").text) # Finds the tag Population, gets its text value and stores it in the variable Population
 Random = (tree.find("Random").text) # Finds the tag Random, gets its text value and stores it in the variable Random
 OSSpecific = (tree.find("OSSpecific").text) # Finds the tag OSSpecific, gets its text value and stores it in the variable OSSpecific
 InHouse = (tree.find("InHouse").text) # Finds the tag InHouse, gets its text value and stores it in the variable InHouse
 Category = (tree.find("Category").text) # Finds the tag Category, gets its text value and stores it in the variable Category
 setTextList = {self.nameLE:Name,self.DescLE:Description, # Creates the dictionary SetTextList
 self.UnlockLE:Unlock,self.PopLE:Population,self.RanLE:Random,
 self.OSSLE:OSSpecific,self.HouseLE:InHouse,self.categoryLE:Category}
 for line,value in setTextList.items(): # Goes through every line and value in setTextList 
 line.setText(value) # Sets the text for the current line to whatever value is
 self.funcName.setEnabled(True) # Enables funcName
 def setNameGenerator(self,tree):
 nameGenerator = (tree.find('NameGenerator')) # Finds the tag NameGenerator
 nameGeneratorChoice = str(self.nameGeneratorLE.text()) # Gets the text of the lineEditor nameGenerator, turns it into a string, and then stores it in the variable nameGeneratorChoice
 nameGenerator.text = nameGeneratorChoice # Gives the text value of nameGenerator the value of nameGeneratorChoice
 def populateFeatureFields(self, tree):
 features = tree.find('Features') # Finds the tag Features and stores it in the variable features
 for functionName,functionKey in self.listOfFeatures.items(): # Goes through each fuctionName and functionKey in the list self.listOfFeatures
 if functionName == self.featureComboBox.currentText(): # Checks if functionName is equal to the current text of featureComboBox
 Description = (features[functionKey].find("Description").text) # Finds the tag Description's text value and stores it in the Description variable
 Unlock = (features[functionKey].find("Unlock").text) # Finds the tag Unlock's text value and stores it in the Unlock variable
 DevTime = (features[functionKey].find("DevTime").text) # Finds the tag DevTime's text value and stores it in the DevTime variable 
 Innovation = (features[functionKey].find("Innovation").text) # Finds the tag Innovation's text value and stores it in the Innovation variable
 Usability = (features[functionKey].find("Usability").text) # Finds the tag Usability's text value and stores it in the Usability variable
 Stability = (features[functionKey].find("Stability").text) # Finds the tag Stability's text value and stores it in the Stability variable
 CodeArt = (features[functionKey].find("CodeArt").text) # Finds the tag CodeArt's text value and stores it in the CodeArt variable
 Server = (features[functionKey].find("Server").text) # Finds the tag Server's text value and stores it in the Server variable
 self.functionKey = functionKey # Stores the value of functionKey in the variable self.functionKey
 featureText = {self.descEdit:Description,self.unlockEdit:Unlock, # Makes the dictionary called feautureText
 self.devtimeEdit:DevTime,self.innovationEdit:Innovation, 
 self.usabilityEdit:Usability,self.stabilityEdit:Stability, 
 self.codeartEdit:CodeArt,self.serverEdit:Server} 
 for line,text in featureText.items(): # Goes through every line and text in the dictionary featureText
 line.setText(text) # Sets the text of the current line to the value of text
 listOfThingsToEnable = [self.descEdit,self.unlockEdit,self.devtimeEdit, # Creates the list listOfThingsToEnable
 self.usabilityEdit,self.stabilityEdit,self.codeartEdit, 
 self.serverEdit,self.innovationEdit] 
 for thing in listOfThingsToEnable: # Goes through each item in the list listOfThingsToEnable
 thing.setEnabled(True) # Enables the selected item
 self.featureToEdit = functionKey # Sets the value of featureToEdit to the value of functionKey
 self.addDependenciesToComboBox(self.tree,self.functionKey) # Executes the function addDependenciesToComboBox
 self.deleteFeatureBttn.setEnabled(True) # Enables the button deleteFeatureBttn
 def addFeature(self,tree):
 name = self.funcName.text() # Finds the text value of funcName and stores it in the variable name
 if name != '': # Checks that name is not equal to an empty string
 features = tree.find('Features') # Finds the Features tag and stores its value in the variable features
 count = len(features) # Gets the size of features and stores it in the variable count
 functionExists = self.checkIfFeatureExists(tree,features,count,name) # Executes the function checkIfFeatureExists and stores the returned value in the variable functionExists
 if functionExists != True: # Checks if functionExists is not equal to True
 new_feature = deepcopy(features[0]) # Makes a copy of the first feature and stores it in the variable new_feature
 for tag in new_feature: # Goes through every tag in new_feature
 tag.text = '' # Sets the text value to an empty string
 new_feature.find('Name').text = name # Finds the tag Name and sets its text value to the value of the variable name
 features.insert(count, new_feature) # Finds the tag name in the feature that will be added and gives it a text value of name
 self.addFeaturesToComboBox(self.tree) # Inserts a new feature
 self.statusBar().showMessage('Feature Created',1500) # Executes the addFeaturesToComboBox function
 elif name == '': # Shows message 'Feature Created'
 em.showNoFeatureNameError(self) # Shows error message for no feature name
 def checkIfFeatureExists(self,tree,features,numberOfFeatures,name): 
 numberOfFeatures -= 1 # Subtracts one from numberOfFeatures and assigns it to numberOfFeatures
 while numberOfFeatures >= 0: # Loop that runs while numberOfFeatures is greater than or equal to 0
 featureName = features[numberOfFeatures].find('Name') # Finds the tag Name in the selected feature and stores it in the variable featureName
 if featureName.text == name: # Checks if the text value of featureName is equal to name
 return True # Returns the value True
 numberOfFeatures -= 1 # Subtracts one from numberOfFeatures and assigns it to numberOfFeatures
 def getFeatureIndex(self,tree,features,numberOfFeatures,name): 
 while numberOfFeatures >= 0: # Loop that runs while numberOfFeatutes is greater than or equal to 0
 featureName = features[numberOfFeatures].find('Name') # Finds the Name tag in the selected feature and stores it in the variable featureName
 if featureName.text == name: # Checs if the text value of featureName is equal to name
 return numberOfFeatures # Returns the variable numberOfFeatures
 numberOfFeatures -= 1 # Subtracts one from numberOfFeatures and assigns it to numberOfFeatures
 def getDependencyIndex(self,tree,dependencies,numberOfDependencies,name):
 numberOfDependencies -= 1 # Subtracts one from numberOfDependencies and assigns the new value to numberOfDependencies
 while numberOfDependencies >= 0: # Loop that runs while numberOf
 dependencyName = dependencies[numberOfDependencies] # Gets the wanted dependency and stores it in the variable dependencyName
 if dependencyName.text == name: # Checks if the text value of dependencyName is equal to the value of the varaible name
 return numberOfDependencies # Returns the value of the variable numberOfDependencies
 numberOfDependencies -= 1 # Subtracts one from numberOfDependencies and assigns the new value to numberOfDependencies
 def addFeaturesToComboBox(self,tree): 
 self.featureComboBox.clear() # Clears self.featureComboBox
 features = tree.find('Features') # Finds features and stores it in the variable features
 numberOfFeatures = len(features) - 1 # Gets the size of features and subtracts one from it, then stores it in the variable numberOfFeatures
 if numberOfFeatures >= 0: # Checks if numberOfFeatures is greater than or equal to 0
 while numberOfFeatures >= 0: # Loop that runs while numberOFFeatures is greater than or equal to 0
 name = features[numberOfFeatures].find('Name') # Finds the tag name in the selected feature and stores it in the variable name
 self.listOfFeatures[name.text]= numberOfFeatures # Creates a key for self.listOfFeatures and gives it the value of numberOfFeatures
 self.featureComboBox.addItem(name.text) # Adds the text value of name to featureComboBox
 numberOfFeatures = numberOfFeatures - 1 # Subtracts one from numberOfFeatures
 listOfThingsToEnable = [self.featureComboBox,self.editFeatureBttn, # Creates a list called listOfThingsToEnable
 self.dependencyComboBox,self.deleteDependencyBttn] 
 for item in listOfThingsToEnable: # Goes through each item in listOfThingsToEnable
 item.setEnabled(True) # Enables the current item
 def addDependenciesToComboBox(self,tree,key):
 self.dependencyComboBox.clear() # Clears the dependencyComboBox
 features = tree.find('Features') # Finds Features in tree and stores it in the variable features
 feature = features[key] # Finds the wanted feature and stores it in the variable feature
 dependencies = feature.find('Dependencies') # Finds dependencies and stores it in dependencies
 try:
 numberOfDependencies = len(dependencies) - 1 # Gets the size of dependencies and substracts one from it, then stores it in the variable numberOfDependencies
 if numberOfDependencies >= 0: # Checks if numberOfDependencies is greater than or equal to 0
 while numberOfDependencies >= 0: # Loop that runs while numberOfDependencies is greater than or equal to 0
 nameOfDependency = dependencies[numberOfDependencies].text # Gets the text of the currently selected dependency and stores it in the variable nameOfDependency
 self.dependencyComboBox.addItem(nameOfDependency) # Adds nameOfDependency to the dependencyComboBox
 numberOfDependencies -= 1 # Subtracts one from numberOfDependencies and assigns the new value to numberOfDependencies
 except: 
 numberOfDependencies = 0 # Sets numberOfDependencies to 0
 def getFeatureInformation(self,tree,features,feature):
 fList = {(features[self.featureToEdit].find("Description")):self.descEdit, # Creates the list flist
 (features[self.featureToEdit].find("Unlock")) :self.unlockEdit,
 (features[self.featureToEdit].find("DevTime")) :self.devtimeEdit,
 (features[self.featureToEdit].find("Innovation")) :self.innovationEdit,
 (features[self.featureToEdit].find("Usability")) :self.usabilityEdit,
 (features[self.featureToEdit].find("Stability")):self.stabilityEdit,
 (features[self.featureToEdit].find("CodeArt")) :self.codeartEdit,
 (features[self.featureToEdit].find("Server")):self.serverEdit,
 }
 return fList # Returns the list flist
 def addDependencies(self,tree,Dependencies): 
 for item,value in self.oslist.items(): # Goes through each item and value in self.oslist
 etree.SubElement(Dependencies,"Dependency", Software = "Operating System").text = value # Add a Dependency for each item each with a text equal to the value associated to each item
 for item,value in self.vislist.items(): # Goes through each item and value in self.vislist
 etree.SubElement(Dependencies,"Dependency",Software = "Visual Tool").text = value # Add a Dependency for each item each with a text equal to the value associated to each item
 for item,value in self.audlist.items(): # Goes through each item and value in self.audlist
 etree.SubElement(Dependencies,"Dependency",Software = "Audio Tool").text = value # Add a Dependency for each item each with a text equal to the value associated to each item
 def clearDependencyLists(self):
 self.oslist.clear() # Clears oslist
 self.vislist.clear() # Clears vislist
 self.audlist.clear() # Clears audlist
 self.customDependencyList.clear() # Clears customDependencyList
 def addCustomDependency(self,key,value):
 if key != '' and value != '': # Checks if key and value are both not equal to ''
 if key in self.customDependencyList: # Checks if key is in self.customDependencyList[key]
 self.checkForValue(value,key) # Executes function checkForValue
 else:
 self.keyIncidentDict[key] = 1 # Creates a key for the dictionary keyIncidentDict and assigns it the value of 1
 self.customDependencyList[key] = value # Creates a key for the dictionary customDependencyList and assigns it the value of the variable value
 else:
 em.showNoSoftwareErrorOrNoFeatureError(self) # Shows message to let the user know he/she did not give a name for the Software or feature
 def checkForValue(self,value,key):
 if value not in self.customDependencyList[key]: # Checks if value is not in self.customDependencyList[key]
 deplist = list(self.customDependencyList[key]) # Turns self.customDependencyList[key] into a list and stores it in deplist 
 deplist.append(value) # Appends value to deplist
 self.customDependencyList[key] = deplist # Creates a key for self.customDependecyList and assigns it the value of deplist
 self.keyIncidentDict[key] = self.keyIncidentDict[key] + 1 # Creates a key for keyIncidentDict and assigns it the value key + 1
 else:
 self.statusBar().showMessage('Cannot add same dependency twice.',1500) # Shows message 'Cannot add same dependency twice.' 
 def addCustomDependencies(self,tree,Dependencies):
 try:
 for key,value in self.customDependencyList.items: # Goes through each key and value in self.customDependencyList
 counter = (len(self.customDependencyList[key])) - 1 # Gets the length of self.customDependencyList[key] and assigns it to the variable 'counter' 
 software = key # Assigns the value key to the variable software.
 if self.keyIncidentDict[key] > 1: # Condition: Checks if self.keyIncident[key] is greater than 1
 self.AddCustomDependencyToTree(key,counter,value,software) # Executes the function AddCustomDependencyToTree
 else:
 etree.SubElement(Dependencies,"Dependency", Software = software).text = value # Adds a Dependency to the file with an attribute of the value of software and with the text of value 
 self.keyIncidentDict.clear() # Clears the keyIncidentDict dictionary
 except:
 em.showUnexpectedError(self)
 def AddCustomDependencyToTree(self,key,counter,value,software):
 while counter >= 0: # Loop: While counter is greater than 0
 software = key # Assigns the value of key to the variable software
 value = self.customDependencyList[key][counter] # Assigns the value of self.customDependencyList[key][counter] to the variable value
 etree.SubElement(Dependencies,"Dependency", Software = software).text = value # Adds a Dependency to the file with an attribute of the value of software and with the text of value
 counter = counter - 1 # Subtracts one from the counter
 def saveChangesToFeature(self,tree): 
 try:
 features = tree.find('Features') # Finds Features in tree and stores it in the features variable
 feature = features[self.functionKey] # Finds feature in features and stores it in the variable feature
 fList = self.getFeatureInformation(tree,features,feature) # Executes the self.getFeatureInformation fucntion and stores the returned valuei n the fList list
 for field,content in fList.items(): # Goes through the contents of fList
 field.text = str(content.text()) # Turns the context's text into a string and stores it in field.text
 Dependencies = (features[self.featureToEdit].find("Dependencies")) # Finds dependencies
 self.dependencyManaging(tree,Dependencies,features) # Executes the dependencyManaging function
 self.checkForAttributes(feature) # Executes the check for Attributes function
 self.enableDependencyBoxes() # Executes enableDependencyBoxes
 self.clearDependencyLists() # Executes the clearDependencyList function
 self.addDependenciesToComboBox(self.tree,self.featureToEdit) # Executes the addDependenciesToComboBox function
 self.statusBar().showMessage('Saved changes to feature',1500) # Shows 'Saved changes to feature' message
 except:
 em.errorWhileSaving(self)
 self.checkFromCheckBoxStatus()
 if self.fromStatus == True:
 feature.attrib['From'] = str(self.fromLE.text())
 def enableDependencyBoxes(self):
 depBoxList = [OSDEPBOX, VISDEPBOX, AUDIODEPBOX] # Makes a list with all dependency boxes
 for depBox in depBoxList: # Goes through every dependency box in depBoxList
 self.depbox.setEnabled(True) # Enables the dependency box
 def dependencyManaging(self,tree, Dependencies, features):
 if Dependencies is None: # Checks if Dependencies is None
 etree.SubElement(features[self.functionKey],"Dependencies") # Adds Dependencies tag to the feature that is being edited
 Dependencies = (features[self.featureToEdit].find("Dependencies")) # Finds Dependencies and stores it in a variable
 self.addDependencies(tree,Dependencies) # Executes the addDependencies Function
 self.addCustomDependencies(tree,Dependencies) # Executes the addCustomDependencies Functions
 else: 
 self.addDependencies(tree,Dependencies) # Executes addDependencies Function
 self.addCustomDependencies(tree,Dependencies) # Executes addCustomDependencies Function
 def checkForAttributes(self,feature):
 if self.vitalStatus == True: # Checks if vitalStatus is True
 feature.attrib['Vital'] = 'TRUE' # Adds the attribute 'Vital' = TRUE to the selected feature 
 if self.forcedStatus == True: # Checks if forcedStatus is True
 feature.attrib['Forced'] = 'TRUE' # Adds the attribute 'Forced' = TRUE to the selected feature
 if self.fromStatus == True: # Checks fromStatus is True
 feature.attrib['From'] = 'TRUE' # Adds the attribute 'From' = TRUE to the selected feature
 def setNewSoftwareTypeValues(self): # Sets new values for software (User must still save for changes to be made)
 Name = self.tree.find('Name') # Looks for Name tag
 Description = self.tree.find('Description') # Looks for Description tag
 Unlock = self.tree.find('Unlock') # Looks for Unlock tag
 Population = self.tree.find('Population') # Looks for Population tag
 Random = self.tree.find('Random') # Looks for Random tag
 OSSpecific = self.tree.find('OSSpecific') # Looks for OSSpecific tag
 InHouse = self.tree.find('InHouse') # Looks for InHouse tag
 Category = self.tree.find('Category') # Looks for Category tag
 setTextDict = {Name:self.nameLE,Description:self.DescLE, # Creates setTextDict
 Unlock:self.UnlockLE,Population:self.PopLE,Random:self.RanLE,
 OSSpecific:self.OSSLE,InHouse:self.HouseLE,Category:self.categoryLE}
 self.setText(setTextDict)
 def setText(self,setText): 
 for field,line in setText.items(): # Goes through a dictionary assigning a text field for each category
 field.text = str(line.text()) # Stores the wanted text in the right tag
 self.statusBar().showMessage('Changes made',1500) # Shows
 def delFunc(self):
 functionToBeDeleted = self.featureComboBox.currentText() # The function currently selected is the function to be deleted
 index = self.featureComboBox.findText(functionToBeDeleted) # Gets the index for the function to be deleted
 self.featureComboBox.removeItem(index) # Removes the function that was selected
 def deleteFeature(self,tree):
 featureToDelete = str(self.featureComboBox.currentText()) # Stores the name of the feature that will be deleted into a variable
 features = tree.find('Features') # Defines features
 numberOfFeatures = len(features) - 1 # Stores the number of features - 1 into a variable
 featureKey = self.getFeatureIndex(tree,features,numberOfFeatures,featureToBeDeleted) # Looks for the index of the feature that will be deleted and stores it in a variable
 features.remove(features[featureKey]) # The feature is removed from features.
 self.delFunc() # The delfunc function is executed and results in the function to be removed from the function combo box
 def deleteDependency(self,tree):
 try: 
 features = tree.find('Features') # Finds features and stores it in a varaible 
 dependencies = (features[self.featureToEdit].find("Dependencies")) # Finds Dependencies and stores it in a varaible
 dependencyName = str(self.dependencyComboBox.currentText()) # Gets the current text in the dependency ComboBox, turns it into a string, and stores it in a varaible
 numberOfDependencies = self.dependencyComboBox.count() # Stores the count from the dependency ComboBox and stores it in a variable
 index = self.getDependencyIndex(tree,dependencies,numberOfDependencies,dependencyName) # Gets the index for the dependency that will be deleted
 dependencies.remove(dependencies[index]) # Using the index from the last step, removes the dependency from the dependencies
 self.removeDependencyFromComboBox(dependencyName) # Removes the dependency from the dependency ComboBox
 except: 
 if self.dependencyComboBox.count() == 0: 
 self.statusBar().showMessage('There are no dependencies to delete',1500) # Shows a message that says 'There are no dependencies to delete' if there are no dependencies in the dependency ComboBox
 else: 
 em.showUnexpectedError(self) # Shows an unexpected error message
 def removeDependencyFromComboBox(self,dependencyName):
 comboBoxDependency = self.dependencyComboBox.findText(dependencyName) # Finds the text given by the dependencyName parameter and stores it in a varaible 
 self.dependencyComboBox.removeItem(comboBoxDependency) # Removes the dependency from the dependency comboBox 
 def checkForDependency(self,testVar,dependency):
 dependencyStatus = None # Creates the variable dependencyStatus and sets it to None
 while testVar > 0: # Creates a loop that runs while testVar is greater than 0
 dependencyStatus = dependency not in self.oslist # 
 if dependencyStatus == True: # Checks if dependencyStatus is True
 testVar = testVar - 1 # Substracts one from the testVar
 elif dependencyStatus == False: # Checks if dependencyStatus is False
 break # Breakes the loop
 return dependencyStatus 
 def addOSDependency(self):
 testVar = self.depco - 1 # Subtracts one from the class variable depco and assigns it to testVar
 dependencySelected = str(self.OSDEPBOX.currentText()) # Gets the current text from the OS Dependency ComboBox, turns it into a string, and assigns it to dependencySelected
 dependencyStatus = self.checkForDependency(testvar,dependencySelected) # Executes the checkForDependency function and stores it into the dependencyStatus variable
 if dependencyStatus != False: # Checks if dependencyStatus is False
 self.oslist[self.depco] = dependencySelected # Makes a key for the dictionary oslist and gives it a value.
 self.depco = self.depco + 1 # Subtracts one from depco and assigns it to depco
 else:
 self.statusBar().showMessage('Cannot add same dependency twice',1500) # Shows a message that says 'Cannot add same dependency twice'
 def addVisDependency(self):
 testVar = self.depco2 - 1 # Subtracts one from depco2 and assigns it to testVar
 dependencySelected = str(self.VISDEPBOX.currentText()) # Gets the current text from the Visual Dependency ComboBox, turns it into a string, and assigns it to dependencySelected
 dependencyStatus = self.checkForDependency(testVar,dependencySelected) # Executes the checkForDependency function and stores it into the dependencyStatus variable
 if dependencyStatus != False: # Checks if dependencyStatus is False
 self.vislist[self.depco2] = dependencySelected # Makes a key for the dictionary vislist and gives it a value.
 self.depco2 = self.depco2 + 1 # Subtracts one from depco2 and assigns it to depco2
 else:
 self.statusBar().showMessage('Cannot add same dependency twice',1500) # Shows a message that says 'Cannot add same dependency twice'
 def addAudDependency(self): 
 testVar = self.depco3 - 1 # Subtracts one from depco3 and assigns it to testVar
 dependencySelected = str(self.AUDIODEPBOX.currentText()) # Gets the current text from the Audio Dependency ComboBox, turns it into a string, and assigns it to dependencySelected
 dependencyStatus = self.checkForDependency(testVar,dependencySelected) # Executes the checkForDependency function and stores it into the dependencyStatus variable
 if dependencyStatus != False: # Checks if dependencyStatus is False
 self.audlist[self.depco3]= dependencySelected # Makes a key for the dictionary audlist and gives it a value.
 self.depco3 = self.depco3 + 1 # Subtracts one from depco3 and assigns it to depco3
 else:
 self.statusBar().showMessage('Cannot add same dependency twice',1500) # Shows a message that says 'Cannot add same dependency twice'
 def dependencyCheckStatus(self):
 checkboxDict = {self.OSCHECKBOX:[self.OSDEPBOX,self.addDependencyBttn], # Creates the dictionary checkboxDict
 self.VISCHECKBOX:[self.VISDEPBOX,self.addVisualDependencyBttn], 
 self.AUDCHECKBOX:[self.AUDIODEPBOX,self.addAudioDependencyBttn]} 
 for checkbox,buttons in checkboxDict.items(): # Goes through the items in checkboxDict
 for button in buttons: # Goes through each button in buttons
 if checkbox.isChecked() == True: # Checks if the checkbox is checked
 button.setEnabled(True) # Enables the button
 else:
 button.setEnabled(False) # Does not enable the button
 def checkForcedCheckBoxStatus(self):
 ForcedStatus = self.FORCEDCHECKBOX.isChecked() # Assigns the value of self.FORCEDCHECKBOX.isChecked() to ForcedStatus
 if ForcedStatus == True: # Checks if ForcedStatus is True
 self.forcedStatus = True # Sets forcedStatus to True
 self.FROMCHECKBOX.setEnabled(False) # Disables FROMCHECKBOX
 self.VITALCHECKBOX.setEnabled(False) # Disables VITALCHECKBOX
 else:
 self.forcedStatus = False # Sets forcedStatus to False
 self.FROMCHECKBOX.setEnabled(True) # Enables FROMCHECKBOX
 self.VITALCHECKBOX.setEnabled(True) # Enables VITALCHECKBOX
 def checkVitalCheckBoxStatus(self):
 VitalStatus = self.VITALCHECKBOX.isChecked() # Assigns the value from self.VITALCHECKBOX.isChecked() to VitalStatus
 if VitalStatus == True: # Checks if VitalStatus is True
 self.vitalStatus = True # Sets vitalStatus to True
 self.FORCEDCHECKBOX.setEnabled(False) # Disables FORCEDCHECKBOX
 else: 
 self.vitalStatus = False # Sets vitalStatus to False 
 self.FORCEDCHECKBOX.setEnabled(True) # Enables FORCEDCHECKBOX
 def checkFromCheckBoxStatus(self): 
 ForcedStatus = self.FROMCHECKBOX.isChecked() # Stores the value from self.FROMCHECKBOX.isChecked() in ForcedStatus
 if ForcedStatus == True: # Checks if ForcedStatus is True
 self.fromStatus = True # Sets fromStatus to True
 self.FORCEDCHECKBOX.setEnabled(False) # Disables FORCEDCHECKBOX
 else:
 self.fromStatus = False # Sets fromStatus to False
 self.FORCEDCHECKBOX.setEnabled(True) # Enables FORCEDCHECKBOX
 def clearThings(self):
 self.oslist.clear() # Clears oslist
 self.vislist.clear() # Clears vislist
 self.audlist.clear() # Clears audlist 
 clearList = {self.descEdit, self.unlockEdit, self.devtimeEdit, self.innovationEdit, 
 self.usabilityEdit, self.codeartEdit, self.serverEdit, self.stabilityEdit}
 for item in clearList:
 item.clear() 
def main():
 app = QtGui.QApplication(sys.argv)
 global form
 form = ExampleApp()
 form.show()
 app.exec_()
if __name__ == '__main__':
 main()
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Jan 11, 2016 at 19:02
\$\endgroup\$
5
  • \$\begingroup\$ Welcome to Code Review. Unfortunately I personally do not consider this a good question (In particular, see the "Don't assume that everyone knows what you are talking about" section, and work on your Description). With a few changes to your question, you can make it much more clearer and more interesting for reviewers. I am hoping to give you an upvote soon. \$\endgroup\$ Commented Jan 11, 2016 at 19:12
  • \$\begingroup\$ I understand. Edited and added more to the description. If it is still seems like it won't be good enough i'd like to know so I can remove it. \$\endgroup\$ Commented Jan 11, 2016 at 19:22
  • \$\begingroup\$ Welcome to Code Review! I have rolled back the last edit. Please see what you may and may not do after receiving answers . \$\endgroup\$ Commented Jan 11, 2016 at 21:06
  • \$\begingroup\$ I only glanced through the code, but I'll say this: whitespace is your friend! It's difficult to read when it's as squished together as it is. Unless you code is minified, it's always better for it to take up more room to enhance readability. \$\endgroup\$ Commented Jan 11, 2016 at 23:45
  • \$\begingroup\$ Please don't replace the title with such a generic one. If this one isn't accurate enough, then you may make it more accurate. \$\endgroup\$ Commented Jan 12, 2016 at 5:47

2 Answers 2

7
\$\begingroup\$

The biggest problem with this code is that I have no idea what it does.

Complicated code is not inherently bad – sometimes we’re just solving thorny problems, and we need an in-depth solution. But it’s very hard for me to work out what this code is doing, and how it’s supposed to work.

The only comments in the program are just telling me what the code does – they should explain why it behaves that way. Docstrings should tell me what functions do, and how to use them. There should be a module doctoring explaining how to use the file. And so on.

Good documentation serves to mitigate complication. If there are comments to explain why it’s been written this way, and to guide me through, it becomes much easier to follow.

It’s hard to say whether the code could be simpler without knowing what it’s supposed to do.


And to add a bit of substance to those bones, here are some specific corrections I can suggest without knowing what the code does.

  • Read PEP 8, particularly the parts about spacing. There are parts where text has been rammed together (newlines between methods, the checkboxDict on line 464, arguments to functions) that just make it harder to read.

    If you run something like flake8 over your code, it will help identify these style/readability issues. It can also help to identify typos (for example – look carefully at addOSDependency() and work out why it will always throw a NameError).

  • There are a lot of checks in your program of the form:

    if variable == True:
     do_thing()
    

    It’s cleaner and more Pythonic to drop the == True part and just write:

    if variable:
     do_thing()
    

  • Towards the end of the script, I see blocks of code like this:

    if ForcedStatus == True: # Checks if ForcedStatus is True
     self.fromStatus = True # Sets fromStatus to True
     self.FORCEDCHECKBOX.setEnabled(False) # Disables FORCEDCHECKBOX
    else:
     self.fromStatus = False # Sets fromStatus to False
     self.FORCEDCHECKBOX.setEnabled(True) 
    

    The repetition isn’t great, and we can simplify the code to get rid of it:

    self.fromStatus = ForcedStatus
    self.FORCEDCHECKBOX.setEnabled(not ForcedStatus)
    

  • Make sure your functions always return a meaningful result. For example, you have a function checkIfFeatureExists, which returns True if it finds a feature with the given name. It returns None if it doesn’t – it would be nicer to return False.

    Speaking of that function, you can simplify the loop it uses like so:

    def check_if_feature_exists(self, features, name): 
     for feature in features:
     feature_name = feature.find('Name') 
     if feature_name.text == name:
     return True
     return False
    

    Notice that I’ve also been able to drop two arguments from the method signature that weren’t being used.

  • Likewise, you can tidy up getFeatureIndex by using Python's enumerate() function – let it do the work of tracking the index variable.

    def get_feature_index(self, features, name): 
     for idx, feature in enumerate(features):
     feature_name = feature.find('Name')
     if feature_name.text == name:
     return idx
    

  • Try not to pass around redundant information – for example, the original getDependencyIndex() takes both dependencies and numberOfDependencies as arguments. You only need the first argument – you can derive the second.

    This ensures you’re not caught out by callers passing bad data.

  • A nitpick, but in the clearThings() method, you have a variable called clearList which is really a set. If you can avoid little inconsistencies like this, your code will be easier to read.

answered Jan 11, 2016 at 19:50
\$\endgroup\$
2
  • \$\begingroup\$ Thank you for the feedback, The code adds the functionality to the GUI. I really found everything you said useful. And will make sure to apply it and fix my documentation as well. Thank you for taking the time to look at it. \$\endgroup\$ Commented Jan 11, 2016 at 20:03
  • 2
    \$\begingroup\$ @Oscar "Adds the functionality to the GUI" is a very generic, broad description. Going forward, it will help you, and people reading your code if you give better descriptions. \$\endgroup\$ Commented Jan 11, 2016 at 23:49
2
\$\begingroup\$

Avoid unnecessary lambdas

lambda : x()

Is the same as:

x

You use expressions of the first kind a lot in your __init__ so I would suggest simplyfying.

answered Jan 11, 2016 at 21:36
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.