My Node class defines a _parse method which separates out the node header, and sends those lines to a _parse_metadata method. This is where the elif chain occurs -- each line of the metadata starts with a tag like "dt=" and I need to recognize each tag and set the appropriate Node object attribute, such as .document_type. (I do not want to rely on the unhelpful names for the tags in the file format, preferring more self-documenting attribute names.)
I've come up with *a* way to use a dictionary dispatch, but I'll wager a great deal it isn't the *best* way.
Here is a minimal illustration of what I have come up with:
<code> class A:
def __init__(self):
self.something = None
self.something_else = None
self.still_another_thing = None
def update(self, data):
for key in metadata_dict: if data.startswith(key): exec('''self.%s = """%s"""''' %(metadata_dict[key], data[len(key):])) # triple quotes as there may be quotes in metadata # values break
metadata_dict = {'something_tag=': 'something', '2nd_tag=': 'something_else', 'last=': 'still_another_thing'}
a = A() print a.still_another_thing a.update('last=the metadata value for the "last=" metadata tag') print a.still_another_thing </code>
<output> >>> None the metadata value for the "last=" metadata tag </output>
So, it works. Yay :-)
But, should I be doing it another way?
Another way to do this is to use dispatch methods. If you have extra processing to do for each tag, this might be a good way to go.
I'm going to assume that your data lines have the form 'tag=data'. Then your Node class might have methods that look like this:
class Node: ... def parse_metadata(self, line): tag, data = line.split('=', 1) try: handler = getattr(self, 'handle_' + tag) except AttributeError: print 'Unexpected tag:', tag, data else: handler(data)
def handle_something_tag(self, data): self.something = int(data) # for example
def handle_last(self, data): try: self.another_thing.append(data) # attribute is a list except AttributeError: self.another_thing = [data]
and so on. This organization avoids any if / else chain and puts all the processing for each tag in a single place.
BTW the try / except / else idiom is used here to avoid catching unexpected exceptions. The naive way to write it would be
try: handler = getattr(self, 'handle_' + tag) handler(data) except AttributeError: print 'Unexpected tag:', tag, data
The problem with this is that if handler() raise AttributeError you will get an unhelpful error message and no stack trace. Putting the call to handler() in an else clause puts it out of the scope of the try / except but it will still be executed only if the getattr succeeds.
Also, I know the general security concerns about things like exec. They make me nervous in using it, even though I am (as yet) the sole user. Am I right in thinking that the constrained way I am using it here protects me? My code uses most of the attributes as a simple storage container for later rewriting of the file, but in a few cases they enter into (safe seeming) conditionals like:
if 'text' == self.document_type: self.do_text_stuff() if 'RTF' == self.document_type: self.do_RTF_stuff()
Conditionals on a 'type' flag are a code smell that suggests using subclasses. Maybe you should have a TextNode class and an RtfNode class. Then the above becomes just
self.do_stuff()
and TextNode and RtfNode each have the appropriate implementations of do_stuff().
I'm not saying this is the right choice for you, just something you might consider.
Kent
Thanks and best to all,
Brian vdB
_______________________________________________ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor
_______________________________________________ Tutor maillist - Tutor@python.org http://mail.python.org/mailman/listinfo/tutor