[MERGE] Clarify use of underscores in HACKING

[John Arbash Meinel]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ian Clatworthy wrote:
> Once more (sorry) with a BB friendly subject line ...
> 
> This patch is a small change to HACKING to better reflect the convention
> used around underscores. I had been using an underscore to mean
> "internal to this module" and a lack thereof to mean "external to this
> module" when Robert explained to me that underscore really means
> "internal to bzrlib".
> 
> As that was a partial surprise, I think we need to make that clearer in
> HACKING.
> 
> Ian C.

...

>  Naming
>  ------
>  
> -Functions, methods or members that are in some sense "private" are given
> -a leading underscore prefix.  This is just a hint that code outside the
> -implementation should probably not use that interface.
> +Functions, methods or members that are "private" to bzrlib are given
> +a leading underscore prefix.  Names without a leading underscore are
> +public not just across modules but to programmers using bzrlib as an
> +API. As a consequence, a leading underscore is appropriate for names
> +exposed across modules but that are not to be exposed to bzrlib API
> +programmers.
>  
>  We prefer class names to be concatenated capital words (``TestCase``)
>  and variables, methods and functions to be lowercase words joined by


I think Aaron had some good points, I would just like to point out a bit
more.

1) Nothing in python can be made truly hidden, unless you write it in an
extension module. Python has 2 ways "_foo" and "__foo". "__foo" actually
gets mangled to try extra hard to make it private. So:

  class Foo:
    def __hidden(self):
       pass
    def func(self):
	return self.__hidden()

  Is actually accessed as f = Foo(); f._Foo_hidden()

2) The biggest problem with __hidden() is that it is completely hidden
inside the class. It is really hard to override in children classes. And
especially with plugins, things you think you want hidden, are often
not. (I've wished I could easily override some of the hidden members of
Thread before).

3) We generally have these classes of functions:

  a) Fully public. This is intended to be a stable function that we will
support with (at a minimum) a deprecation period.

  b) Meant to be used, but not guaranteed to be stable. _iter_changes is
the most notable function in this category.

  c) Accessible between bzrlib modules, but not really intended to be
used by plugins, etc.

  d) Intended to be only used within a class (not meant to be seen by
other bzrlib classes)

  e) Not intended to be used by anyone or anything at any time, we
didn't even realize that was there. :)



Category (b) exists because of our performance drive. We have an api
which we *think* will fit what we want, but we aren't going to guarantee
it, because we may need to tweak it to get it to work properly for
performance. (If you end up needing extra information that you have to
loop over the whole tree to get, then we need to revise the function, as
it should be the only thing iterating the whole tree).

Also, for _iter_changes() we generally have public-facing functions like
Tree.changes_from(other_tree) which use _iter_changes() as the workhorse.

(a) is pretty obvious

(c) Has to do with 'friend' classes (to use the C++ term). These are
functions that are only really meant to be used by close relatives,
(like RevisionTree possibly accessing Repository.get_weave() so that it
can get the text for a file). But get_weave() isn't really meant as a
public for everyone. (Especially now that we are trying to deprecate the
VersionedFile view of the world).

(d) Also generally obvious.

I don't think we have many (e).


Now, because of point (1) and (2), we don't really want to use the
'__foo' forms for functions. It also makes them very hard to whitebox
test, because of the name mangling.

Which gives us a single character to indicate 4 different types of
functions. And 2^1 < 4. We could possibly add another character into the
 mix. But you start getting into Hungarian notation pretty quickly.

Otherwise, +1 on your patch. But you may want to take this discussion
into account.

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iD8DBQFGnjmpJdeBCYSNAAMRAsCWAJ9NnsHa5URI/UoiShhVWKGCI6cyKgCeJBwc
WY4UTWLGzO+2bygnDYtX+q4=
=Sy4L
-----END PGP SIGNATURE-----