Return to Answer

• 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.

• 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.

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

• 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.

• 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.

• 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.

• 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.

• 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.