Re: Private library symbols...

[Richard Frith-Macdonald]

On 16 Oct 2006, at 04:13, Sheldon Gill wrote:

> Hi Richard,
>
> You've made some recent changes to base with the idea of making  
> private functions in base more obviously private and less accessible.

Yes ... as obviously private as possible.

> It seems to be that there is really only one goal with these changes:
>
>   applications/tools shouldn't be using private functions in base
>
> Is there any other objective that I'm missing?

Well, that's the main tactical objective ... the strategic objective  
is to improve maintainability ... so centralisation and clarification  
internally is almost as much of a goal as simply trying to ensure  
that external apps/tools don't use private stuff.

> If this is the case, I think the approach being taken here isn't  
> the one which we should be pursuing.
>
> The original situation is we have functions like these:
>
> NSString  *GSLastError(int);
> BOOL       GSEnvironmentFlag( const char *, BOOL );
>
> which make these functions seem like GNUstep API additions but  
> really they are private.
>
> What has been done is create an "artificial" object
>
> @interface _GSPrivate : NSObject
> + (NSString*) error;
> @end
>
> @interface _GSPrivate (ProcessInfo)
> + (BOOL) environmentFlag: (const char *)name defaultValue: (BOOL)def;
> @end

Yes, the idea is to collect everything together as much as possible,  
making it easier to find these things and making it clear that the  
various mechanisms are used internally.

> Now I say artificial because it doesn't conform to an object  
> design. There never is an actual instantiation, for example, so  
> you're always sending messages to the class. There are many other  
> things which make this not really an object.

This is a work in progress ... I'm not sure yet whether it would be  
good to have an instance and use instance methods rather than just a  
class and use class methods.  The main reason for using a class is to  
use language features to support encapsulation and maintainability  
rather than depending solely on conventions.

> Doing this adds many more bytes to the library.
> It also makes these method lookups rather than function calls so  
> they are slower.

True, but these are very minor overheads and I think clarity and  
maintainability come first before looking at optimisation (obviously  
we can optimise by caching method IMPs or by using a struct as a  
dispatch table rather than using a class).

> So we've added an artificial/strange object, increased the library  
> size and slowed down those function calls all in an effort to  
> prevent applications using our private functions. Is this right?

As you can tell from my answers above, it's only partially right.

> Now its long been idomatic to use the underscore to mark private  
> symbols. In fact, this is precisely what you've done with the class  
> name.

Yes, I've tried to use every signal I could think of  ... the leading  
underscore, the word 'Private' the exclusion from any external  
headers and reduced linker symbols, and the inclusion of comments.

> Further, I think this makes navigating the source harder. Quick,  
> tell me without searching where you're going to find the  
> implementation for the above two functions?

I'm very surprised at this comment ... since a big part of the intent  
is to make navigating the source *easier* and I believe this is  
achieved.
I certainly think it's not obvious where the old functions were ...  
but the new methods are clear since the category names tell you where  
the individual method implementations in each group are, the  
declarations are all in a single place, and you can find all the  
private methods implementations easily using a search for  
@implementation _GSPrivate.  That's a big improvement over the older  
code.

> Why don't we simply change the functions to conform to idomatic norms?
>
> NSString  *_GSLastError(int);
> BOOL       _GSEnvironmentFlag( const char *, BOOL );
>
> and be done with it?

Well, it fails to centralise and make it easier to find things.   
It''s not as clear (in fact meaningless to people not familiar with  
the underscore), and it leaves these as global symbols (easier for  
people to use externally).

> This won't add to size.
> This won't slow things down.
> This stays in convention.
>
> If you really want, we could further decorate along the lines of:
>
> NSString  *_private_GSLastError(int);
> BOOL       _private_GSEnvironmentFlag( const char *, BOOL );
>
> although I don't see the point of doing that. I do see some merit  
> in making these private functions GNU convention
>
> NSString  *_gs_last_error(int);
> BOOL       _gs_environment_flag( const char *, BOOL );
>
> as that is keeping with the style guide.

I think the GNUstep style guide allows you to do what you .like with  
private functions, but consistency with the rules for public ones  
would suggest that the embedded underscores in the above examples  
ought to be replaced with capitalised letters ...  
_privateGSLastError, _privateGSEnvironmentFlag, _GSLastError, and  
_GSEnvironmentFlag.
I guess it's debatable whether we are talking about private or public  
functions here ... they are certainly intended to be private, but as  
long as they are available for external applications to link to, they  
are in at least some sense public.

> Further, if the problem is, as you say, "symbol pollution" why not  
> stop the pollution where it really is occurring: the exports symbol  
> table? Its a linker problem, not really a source code problem.

To be honest, that simply didn't occur to me, as I don't know how to  
do it (or even that it could be reasonably/portably done).
What would be involved in this?  If we *can* remove symbols  
effectively without breaking the library's internal use of them or  
impairing debugging, it would probably be good to do so.