Bug 30649

Short term, the patch looks fine and it's better than not having it.

Longer term, one way to do it would be to have some sort of schema. I hear the cool kids all use RELAX NG these days, but W3C XML Schema would also work. DTDs aren't really suitable since they're not namespace-aware.

Great idea.

How about:

  expected_elts = set(
      (XMLNS_DBUS, 'method'),
      ...,
      (XMLNS_TP, 'property'),
      ...)
  surprising_children = [
      x for x in dom.childNodes
        if (x.namespaceURI, x.localName) not in expected_elts
      ]

  for x in surprising_children:
      ...

I'm not sure if it's any clearer.

I think it might be clearer to put the check elsewhere. You could change the stuff from 'build lists of methods, etc...' onwards in Interface.__init__() to iterate self, and have a despatch table for each supported element name, crying about any unrecognised ones. (The code currently uses getElementsByTagNameNS() which searches all descendants, not just immediate children, but I'm a little suspicious of that anyway. ☺)

+                dom.getElementsByTagNameNS(XMLNS_TP, 'causes-havoc') +

causes-havoc is an attribute, I think.

(In reply to comment #2)
> How about:
[snip]
> I'm not sure if it's any clearer.

I think it's a bit nicer, yes, and has less repetition. I think your second suggestion might be nice but for now I'm going to avoid lots of big changes if that's okay? :-)

> causes-havoc is an attribute, I think.

Yes good point, removed.

PEP 8 says:

> The preferred place to break around a binary operator is
> *after* the operator, not before it.

So this part of your comprehension is bad and wrong:

+            if isinstance(x, xml.dom.minidom.Element)
+            and (x.namespaceURI, x.localName) not in expected

It should be:

+            if isinstance(x, xml.dom.minidom.Element) and
+               (x.namespaceURI, x.localName) not in expected

(lining up the second condition with the first one).

Fair point about the second suggestion.

(In reply to comment #4)
> PEP 8 says:

PEP 8 is balls.

Updated anyway.

Yeah!!!, this is the right architecture for software!!!!!!!

kthxbye