Re: [OVC-demo-team] Fashionably late: BRP beta

From: David Mertz <voting-project_at_gnosis_dot_cx>
Date: Thu Mar 18 2004 - 00:23:20 CST

> I apologize for a long delay in getting this out. Testers, fire up your
> interpreters! There's a lot I need to explain about this, as it is a
> fairly
> complex piece of code, but you'll have to wait until tomorrow for that
> http://gnosis.python-hosting.com/voting-project/OVC-Demo/0274.html
> <wizard.py>

I haven't -really- looked at it yet, mostly just curious about your use
of my minor contribution 'equiv.py'.

However, I found the use of 'equiv' as a namespace modifier for
'gnosis' slightly odd. It -does- work, since 'equiv.py' indirectly
imports gnosis.xml.objectify. But it's more straightforward to just
use the objectify package explicitly. FWIW, I've also uploaded
equiv.py into CVS. So you could make these changes for clarity:

------ At top -----
import sys, os
# to try block --> import equiv # import like this to make it work
from qt import *
from qttable import *
try:
     from evm2003.utils.getxml import ballotxml
     from evm2003.utils.convert import digits2votes, votes2digits,
obscure
     import evm2003.utils.equiv # Use "import ..." not "from ...
import ..."
     from gnosis.xml.objectify import make_instance
except [...]
-------------------

Then in the function reconcileBallot(), the use of objectify can be
simplified quite a bit (and becomes better self-documenting):

----- Line 897 -----
         for ballotFileName in ballotFileNames:
             dirHandler.setCurrent(self.data['appRoot']+'/stored')
             storedBallot = make_instance(ballotFileName)
             dirHandler.setCurrent(self.data['appRoot']+'/scanned')
             try:
                 scannedBallot = make_instance(ballotFileName)
                 if storedBallot == scannedBallot:
                     [...]
--------------------

That is, the name 'make_instance()' is pretty much as self-documenting
as are 'digits2votes()' or 'obscure()', so you might as well leave out
all the namespace prefixes with a "from ... import ...". The other
part is that make_instance() is perfectly happy to work with a file
handle or a filename (and also with an XML unicode string... or even a
DOM object built by another parser). Pervasive polymorphism at work.
 From a readability perspective, I think it looks better without all
those open()'s and read()'s though.

I'm not quite sure about QDir(), I guess it just changes current
directories (maybe with some error catching?). It seems slightly
error-prone to me to switch directories to open files a bunch of times.
  Not so much that filesystems aren't very reliable as that it's easy to
mess up the flow by losing or duplicating one of those .setCurrent()
lines. Stylistically, I would probably have written the above fragment
as:

----- Line 897 -----
         dirHandler.setCurrent(self.data['appRoot']) # But we did this
a few lines up
         for ballotFileName in ballotFileNames:
             storedBallot = make_instance('stored/'+ballotFileName)
             try:
                 scannedBallot = make_instance('scanned/'+ballotFileName)
                 if storedBallot == scannedBallot:
                     [...]
--------------------

If you're really worried about platform issues, you could use 'os.sep'
in place of the '/'. But I think even recent Windows versions are OK
with the forward slash, at least when used via Python. I cannot test
that fact though, since I don't use Windows.

However, despite my slight quibbles, the code is certainly looking very
good!

Yours, David...
==================================================================
= The content of this message, with the exception of any external
= quotations under fair use, are released to the Public Domain
==================================================================
Received on Thu Apr 1 02:40:28 2004

This archive was generated by hypermail 2.1.8 : Thu Apr 01 2004 - 02:40:36 CST