HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Parsing XML commands and parameters

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
commandsxmlparsingandparameters

Problem

I'm C# programmer and started Python recently for a new project.

This is a part of my project, and I want your opinion about my code and if it's clean or dirty, too long, has duplicate code, or it's good and standard.

(cmd= command ... at the end of file is just for class testing)

```
import xml.etree.ElementTree as ET

#TODO: processing attributes should be case insencitive

class command:
def __init__ (self,htmlcommand):
self.preview=False
self.members=[]
self.__parse__(htmlcommand)

def __parse__(self,htmlcommand):
root = ET.fromstring(htmlcommand)
self.service=root.attrib['service']
self.name=root.attrib['name']
self.source=root.attrib['source']
self.preview=bool(root.attrib['preview'])
self.procedurename=root.attrib['procedurename']

mems=root.findall('member')
if(len(mems)==0):
raise Exception('command has no member')
for m in mems:
self.members.append(member(m))

class member:
def __init__ (self,member):
self.params=[]
self.__parse__(member)

def __parse__(self,member):
self.name=member.attrib['name']
self.method=member.attrib['method']
try:
self.order=member.attrib['order']
except:
pass
try:
self.sort=member.attrib['sort']
except:
pass
self.__parseparams__(member)

def __parseparams__(self,member):
params=member.findall('./params/param')
for p in params:
self.params.append(param(p))

#def __getparams(member):

class param:
def __init__(self,param):
self.name=param.attrib['name']
self.value=param.attrib['value']

cmd= command("""







Solution

Multiple things to fix:

  • variable naming - class names should be in camel-case and start with a capital letter (reference); when there are multiple words in a variable, you should put underscore between them, e.g. in your case procedurename would become procedure_name



-
missing spaces - there should be spaces around operators, like around the = here:

self.name = param.attrib['name']


-
incorrect private method naming - when you put double underscores around a method name (this is called "dunder"), you are implying that it is a "magic method" which may confuse future readers of your code (which can also be you); while, I think you meant to create a private method. In other words, naming the method __parse, not __parse__

-
if(len(mems)==0): can be simplified as if not mems:

-
bare except clause should be avoided, either replace with handling a more specific error type:

try: 
    self.order=member.attrib['order']
except:
    pass


with:

try: 
    self.order = member.attrib['order']
except KeyError:
    pass


Or, follow the @LaurentLAPORTE's advice and use the .get() method to set the self.order to None if the "order" attribute does not exist:

self.order = member.attrib.get('order')

Code Snippets

self.name = param.attrib['name']
try: 
    self.order=member.attrib['order']
except:
    pass
try: 
    self.order = member.attrib['order']
except KeyError:
    pass
self.order = member.attrib.get('order')

Context

StackExchange Code Review Q#155428, answer score: 6

Revisions (0)

No revisions yet.