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

Translate strings from an XML attribute into corresponding constants in my module

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

Problem

I have a non-trivial list comprehension in Python. My goal is to take a whitespace-delimited list of strings from an XML attribute — which may not exist — and look up the corresponding constant in my module — which may not exist — and create a set of the results.

What's the accepted way to format such a beast? They all make me want to vomit a little.

self.types = frozenset(
        v for v in (
            getattr(objects_module,enum_name,None)
            for enum_name in element.attrib.get('object-types', '').split())
        if v)


self.types = frozenset(
        v
        for v in (getattr(objects_module,enum_name,None)
            for enum_name in element.attrib.get('object-types', '').split())
           if v)


self.types = frozenset(
        v for v in (
          getattr(objects_module,enum_name,None)
          for enum_name in element.attrib.get('object-types', '').split()
        ) if v
    )


self.types = frozenset(v for v in (getattr(objects_module,enum_name,None) for enum_name in element.attrib.get('object-types', '').split()) if v)

Solution

Well, that's more or less of a personal preference. I'd just adapt any of those to fit my existing code.

For the sake of example, I'd also consider the below example:

self.types = frozenset(v for v in (getattr(objects_module, enum_name, None) for enum_name in
                                   element.attrib.get('object-types', '').split()) if v)


That's just how PyCharm suggested it. But as I said, is more or less a mater of preference. If you want, from what you showed, I'd go with the first one.

You might also consider the following:

split_element = element.attrib.get('object-types', '').split()

self.types = frozenset(v for v in (getattr(objects_module, enum_name, None) for enum_name in split_element) if v)


Which is exactly as your last solution. I've just created an extra variable which will hold the split() part. Now that that respects the PEP8 requirement about the length of a line, I like it.

Code Snippets

self.types = frozenset(v for v in (getattr(objects_module, enum_name, None) for enum_name in
                                   element.attrib.get('object-types', '').split()) if v)
split_element = element.attrib.get('object-types', '').split()

self.types = frozenset(v for v in (getattr(objects_module, enum_name, None) for enum_name in split_element) if v)

Context

StackExchange Code Review Q#145144, answer score: 4

Revisions (0)

No revisions yet.