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()
\$\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\$
    – Vogel612
    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\$
    – Jamal
    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.

\$\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.

\$\endgroup\$

Your Answer

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

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