[Spambayes-checkins] spambayes/spambayes Options.py,1.22,1.23

[Anthony Baxter]

[Tony - please do something with your mailer to keep lines under 80
columns]

>>> "Meyer, Tony" wrote
> > >         self.__sections = self._ConfigParser__sections
> > >         self._ConfigParser__read = self.__read
> > 
> > This is a gross hack.  Private attributes should be private.
> 
> Well, that depends, really.  If private attributes are meant to be 
> private, then they shouldn't be accessable.  From what I have read,
> Python is designed to allow them to be non-private, if necessary.  
> Essentially, they aren't private, they're hidden.

The python name-mangling is meant to indicate "private implementation
specific details". Messing with the internal implementation details of
a class results in code that breaks if the other class changes. 

The basic python rule is something like "we're all grownups", and this 
is why it doesn't enforce data hiding. The flipside to this is that you 
shouldn't be messing with internals. If a class doesn't expose the 
interface you need (particularly if it's in the python std library),
consider posting a patch to the python bug/patch tracker to get this
fixed. 

> * If ConfigParser was well written, IMO, then the read section should
> be protected, not private. There is nothing about it that suggests
> that it shouldn't be overwritten by a subclass, and, in fact, it is
> probably the most likely candidate. __sections shouldn't have had to
> be updated - if the subclass didn't have __sections, then the base
> class's should have been found - again, this should be a protected
> attribute, *not* private. There's no point writing classes that can't
> usefully be extended.

See above - that the ConfigParser didn't expose this interface is 
probably just an indication that no-one had needed to do this before.
There's been a lot of changes in it for Python2.3, so it seems like
you're not the first person to run into this.

In a perfect world, the python std lib would be, well, perfect. Alas,
it is coded by mere humans, so we have to gradually approach perfection
<wink>.

> * The other option was to simply include all the ConfigParser code in 
> UpdatableConfigParser, which would have worked, have resulted in nicer 
> code in many places, and have avoided this.  It seemed a waste though, 
> and against OOP principles.

If the 2.3 ConfigParser class is better, there's nothing saying we can't
include it in the package (we already do this with the sets and heapq
module).

Anthony
-- 
Anthony Baxter     <anthony at interlink.com.au>   
It's never too late to have a happy childhood.