api stability

[Robert Collins]

On Thu, 2006-01-05 at 19:37 +1100, Martin Pool wrote:
> On Thu, 2006-01-05 at 17:18 +1100, Robert Collins wrote:
> > On Thu, 2006-01-05 at 16:38 +1100, Martin Pool wrote:
> > > On Tue, 2005-12-20 at 13:56 +1100, Robert Collins wrote:
> > > > On Mon, 2005-12-19 at 20:28 -0600, John Arbash Meinel wrote:
> > > 
> > > > 'symbol' : Is a public symbol.
> > > > '_symbol' : Is a private symbol to whatever context it appears in. If
> > > > that context is publicly extensible then its public [for extensions
> > > > only].
> > > > Specifically, _foo symbols in module scope are private.
> > > >               _foo symbols in class scope where the class
> > > >                    is named like 'Foo' are public for child classes.
> > > >               _foo symbols in class scope where the class is named 
> > > >                    like '_Foo' are private because the class is 
> > > >                    private and extension is not publically supported.
> > > 
> > > This is OK but I'm not totally happy with it: there is a bit of a gap
> > > between __name (which Python tries to hide from all subclasses), and
> > > allowing things to be used by subclasses.  In particular this implies
> > > that data fields can be relied upon by subclasses, which is going too
> > > far.
> > 
> > Uhm, this is confusing to me: we had __foo for things that are not
> > allowed to be used by subclasses, and _foo for things that are allowed
> > to be used by subclasses. If you have a data field that is not allowed
> > to be relied upon by the subclass, call it __foo.
> 
> My point is there is an important difference between subclasses defined
> inside bzrlib and others.  The practical point is that subclasses
> defined in bzrlib will be tested when the change is made and updated at
> the same time, and so kept in sync.  External subclasses will not be, so
> need a consistent interface.

Yes, and the proposal accomodated that.

> > > Perhaps we should change this to just say that only 'symbol' names can
> > > are public, even when subclassing.  So classes that are intended to be
> > > subclassed in particular ways need to expose with unprefixed names the
> > > symbols needed to define and implement the subclass.
> > 
> > I dont think so, that would force the exposure of class internals in
> > common subclass cases.
> 
> Well, it would mean making no distinction between interfaces which a
> bzrlib client can call from anywhere, and interfaces which the client
> should call only from subclasses of that class.  I think whether it's
> public or not is more important in this context than whether the caller
> is meant to be a subclass.

Uhm I'm seriously confused here. The proposal accomodated *both*
distinctions. I'd like to understand why you want to not make each
statement appropriately in the code base.

> > > I would think only some particular classes need to be subclassed, and
> > > it's better to think about which behaviours should be guaranteed, rather
> > > than potentially promising that none of the internal behaviour will
> > > change.
> > 
> > Eh? I'm *positive* you misread something. That was not implied or stated
> > at all.
> 
> There are some members which are to do with the implementation of a
> class, which need to be accessed inside bzrlib and not just from within
> the class itself, but should not be relied upon by users of bzrlib, even
> if they're making a new subclass.  These are fairly common and
> important: many or most data fields are like this.

Erm, eh? That sounds like bad programming to me, standard data
encapsulation hides such implementation details.

> The scheme you describe above has no way to mark these other than by
> putting them a class whose name starts with _.  It's certainly possible
> to do that (a bit like the C++ 'impl' pattern) but it seems like it will
> contort the code unnecessarily.

So for clarity: you are asking for a means to say 'this is an internal
of a class implementation; bzrlib outside of the class is allowed to
poke at this directly, but code outside of bzrlib is not' ?

Personally, I dont think we *should* have any symbols which that applies
to inside bzrlib anyway *because* it makes our code hard to change and
flex over time. However, if you feel this is a critical enough feature
of our programming style that we want that, we could do something like
'_bzrlib_foo' as a prefix to indicate such a leaky-field. I do object to
your categorisation of fields needing this as fairly common and
important: I think data fields are either part of a contract or not, and
if they are not part of the contract, to allow bzrlib to grow and remain
easy to use data fields that are not part of the contract should not be
accessed by bzrlib: any access to a ._foo field outside of self._foo is
harmful to maintenance and scaling.

> > Sure - the point was that non BzrException subclasses would *never* be
> > raised as part of our api.
> 
> Yes, if you by that you mean "raising them is never part of our
> contract".  But there are some exceptions where that will make sense in
> particular cases - StopIteration, KeyError etc.

Well, as per the prior discussion, raising builtin exceptions like
KeyError should be done with extreme caution anyway. But yes, I can see
needing to do it from time to time.

Rob

-- 
GPG key available at: <http://www.robertcollins.net/keys.txt>.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
Url : https://lists.ubuntu.com/archives/bazaar/attachments/20060105/4de1c8a3/attachment.pgp