Reading XML files and finding pairs between the files

Question

I read three files and parse a few individual fields from each row of the XML.

Using these fields, I find the dictionaries that contain the same keys (ItemK and either SensorK for sensors or PointK for points)

I then append the masterkey and Sensor/Point ID to a new list.

Is there any improvements i can make?

from xml.dom import minidom
mkey = "ConfigMaster.xml"
sensors = "DroppedSensors.xml"
points = "DroppedCustomPoints.xml"
# create the dictionary
masterSensor = [dict.fromkeys(['MasterKey', 'ItemType', 'ItemK'])]
masterPoint = [dict.fromkeys(['MasterKey', 'ItemType', 'ItemK', 'SubItemK'])]
sensor = [dict.fromkeys(['SensorK', 'SensorId'])]
point = [dict.fromkeys(['PointK', 'PointId'])]
# commented the xml write outs
# create point and sensor docs
"""
docSensor = minidom.Document()
docPoint = minidom.Document()
baseSensor = docSensor.createElement("Rows")
basePoint = docPoint.createElement("Rows")
docSensor.appendChild(baseSensor)
docPoint.appendChild(basePoint)
"""
# config master
docMaster = minidom.parse(mkey)
rows = docMaster.getElementsByTagName("Row")
# read each row and add it to the dictionary
for row in rows: 
 mk = row.getAttribute("MasterK")
 it = row.getAttribute("ItemType")
 ik = row.getAttribute("ItemK")
 sik = row.getAttribute("SubItemK")
 if it == 'TSensor' and (sik == '1') :
 masterSensor.append({'MasterKey': mk, 'ItemType': it, 'ItemK':ik}) 
 #tempChild = docSensor.createElement("Row") 
 #tempChild.setAttribute("ItemType", it)
 #tempChild.setAttribute("ItemK", ik)
 #tempChild.setAttribute("MasterK", mk)
 #baseSensor.appendChild(tempChild)
 elif it == 'TCustomPoint' and (sik == '1' or sik == '2') :
 masterPoint.append({'MasterKey': mk, 'ItemType': it, 'ItemK': ik, 'SubItemK': sik}) 
 #tempChild = docPoint.createElement("Row")
 #tempChild.setAttribute("MasterK", mk)
 #tempChild.setAttribute("ItemType", it)
 #tempChild.setAttribute("ItemK", ik)
 #tempChild.setAttribute("SubItemK", sik) 
 #basePoint.appendChild(tempChild)
 #else:
 #print ("Didnt use %s -> %s" % (it, ik)) 
 #print ("MasterKey: %s, ItemK: %s" % (mk, ik))
""" 
docSensor.writexml(open("MasterSensor.xml", 'w'),
 addindent=" ", newl='\n')
docPoint.writexml(open("MasterPoint.xml", 'w'),
 addindent=" ", newl='\n')
docSensor.unlink()
docPoint.unlink()
"""
# ------------------ Sensors -------------------------
# create doc sensor
"""
docWriteSensor = minidom.Document()
base1 = docWriteSensor.createElement("Rows")
docWriteSensor.appendChild(base1)
"""
# sensors 
docSensor = minidom.parse(sensors)
rows = docSensor.getElementsByTagName("Row")
# read each row and add it to the dictionary
for row in rows: 
 sk = row.getAttribute("SensorK")
 idt = row.getAttribute("Ident") 
 sensor.append({'SensorK': sk, 'SensorId': idt})
 #tempChild = docWriteSensor.createElement("Row")
 #tempChild.setAttribute("SensorK", sk)
 #tempChild.setAttribute("Ident", idt) 
 #base1.appendChild(tempChild)
"""
writexml(open("D-Sensors.xml", 'w'),
 addindent=" ", newl='\n')
docWriteSensor.unlink()
"""
# ------------------ Sensors -------------------------
# ------------------ Points -------------------------
# create doc sensor
"""
docWritePoint = minidom.Document()
base2 = docWritePoint.createElement("Rows")
docWritePoint.appendChild(base2)
"""
# points
docPoints = minidom.parse(points)
rows = docPoints.getElementsByTagName("Row")
# read each row and add it to the dictionary
for row in rows: 
 pk = row.getAttribute("PointK")
 idt = row.getAttribute("Ident")
 point.append({'PointK': pk, 'PointId': idt})
 #tempChild = docWritePoint.createElement("Row")
 #tempChild.setAttribute("PointK", pk)
 #tempChild.setAttribute("ItemK", idt) 
 #base2.appendChild(tempChild)
"""
writexml(open("D-Points.xml", 'w'),
 addindent=" ", newl='\n')
docWriteSensor.unlink()
"""
# ------------------ Points -------------------------
# remove initial points
del masterSensor[0]
del masterPoint[0]
del sensor[0]
del point[0]
tcount = 0
tb = False
for item in masterPoint:
 if tb:
 if item['SubItemK'] == '2':
 tcount += 1
 else:
 tb = False
 else:
 if item['SubItemK'] == '1':
 tb = True
print ("MasterKSensors %s\tMasterKPoints %s\tSensorK %s\tPointK %s" % (len(masterSensor),
 len(masterPoint),
 len(sensor),
 len(point)))
print ("Tcount %s" % tcount)
newDict = []
newDict2 = []
for ik in masterSensor:
 for sk in sensor:
 if ik['ItemK'] == sk['SensorK']:
 newDict.append({'MasterKey' : ik['MasterKey'],
 'SensorId' : sk['SensorId']})
 #sensor.remove(sk) # would this work?
 #masterSensor.remove(ik) #
for ik in masterPoint:
 for sk in point:
 if ik['ItemK'] == sk['PointK']:
 newDict2.append({'MasterKey' : ik['MasterKey'],
 'PointId' : sk['PointId']})
 #sensor.remove(sk) # would this work?
 #masterSensor.remove(ik) #
for i in newDict:
 print ("mk %s\tsid %s" % (i['MasterKey'], i['SensorId']))
for i in newDict2:
 print ("mk %s\tpid %s" % (i['MasterKey'], i['PointId']))

• Peilonrayz: What are the multi-line strings? Are they used for documentation or are they old bits of code?
• SuperBiasedMan: @JoeWallis They look like commented out blocks to me.

Answer 1

First of all, you should really be putting things in functions. Your code is a long stretch of code. None of it is separated into chunks to be functions. Why does having functions matter?

• Functions keep code neater because you can see how one block does one specific job.
• They make it easier to test, because you can ensure that each individual function is actually doing it's job right.
• It's easier to reuse a function than it is to copy and paste big blocks of text.

You should to find ways to make your code into functions for readability's sake at least.

You also need to use good naming. newDict is bad enough, but newDict2 tells me even less. Should I see these as similar, or related? What are they for, what do they do and what do they contain? Good variable names describe the abstract use of the variable, and can give an indication to the contents if it's not clear from context. newDict tells me nothing about what it's for. What's worse is, these aren't dictionaries! I often tell people not to put data types in names as it's usually noise, but it's much worse to put inaccurate type names in. At the very least, you could call it dicts, as the s will clearly signal more than one, indicating a list or similar structure which contains dicts.

I would say the same about clarity for mk, it, ik, sik. These may be more specific but they're not explicit about what they are. A user will have to scroll back and re-read them to ever know, it's better to use the extra characters and make them clearer.

Also all your commented lines make it much harder to read. If a comment isn't explaining the code it should be removed for the sake of readability and easier comprehension of what is happening.