hyndgrinder
commented
6 years ago Agreed on all points. Almost all of this is inexperience on my part. In some cases they got weird names because of my poor refactoring (like calling Query Q) when I moved the classes, and then struggled with the imports. In other cases, I made the change to 'element' somewhere down my code, and decided to stick with it, but didn't go back fix it everywhere.
Will try to be more consistent.
Just making this "issue" to ensure that we keep things consistent and readable as we work.
When I glance at the Util class, it's got some confusing stuff in it right now:
-detFilter and detFiltIter look very similar at a glance -It was unclear at a glance what DF was, only because I expected it to be referred to earlier in the method rather than a short-form for an imported class -qry = Q.Query (I'm just not sure we gain anything by making names shorter like that)
It may make sense to make it more consistent across methods. I propose that we name the "iter" variables the same way each time, like "itemGroup" or something, and use something like "for item in itemGroup".
That way we avoid having to find shorter names for "Iter" variables and strange looking names. It also makes copying code much simpler. Since we're going to be parsing XML a lot, being able to copy the structure of a parsing method and not have to change all the variable names is convenient.
I'm a bit torn on this, but it might make sense to just use append() directly on Q.Query() rather than assigning it to a variable first.
Additionally, I think that when parsing XML, we have different argument names for the same thing ("xmlSpec" and "element"). I prefer "element" personally, since we're not looking for textual XML there.