Richard Frith-Macdonald wrote:
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.
Okay.
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.
Well, you actually are relying on conventions anyway. You've called the class
_GSPrivate using the underscore convention to indicate privacy.
Also, these are private function calls within the library. Encapsulation
doesn't really apply here.
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).
I'm quite aware of how we can optimise method calls but this isn't really
optimisation.
I also find this inconsistent with your previous statements about avoiding
'bloat', as you put it. Surely more size and less speed for no functional
changes is the very essence of 'bloat'?
I don't agree that these changes make anything more maintainable or easier than
simply decorating the function calls with an underscore in the conventional way.
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.
Making sure all declarations are in GSPrivate.h is good but a matter of
programmer discipline and has no bearing on the function call vs class method
issue.
> That's a big improvement over the older code.
Conventional layout has private functions near the top of the source file, just
after header includes. They aren't that hard to find. Especially if you keep to
convention with the underscores.
Sure, the category names help you find where the particular function is
located. I've an equivalent way which adds precisely 0 bytes to the libnrary
and makes no impact on the runtime:
// from NSProcessInfo
BOOL _environment_flag(const char *, BOOL );
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).
How does it fail to centralise? The implementations can be where ever they need
to be, including in a single 'centralised' file. You can 'centralise' the
declarations in GSPrivate.h
As to the leading underscore, that is used by C and Objective-C and extensively
within -core. Who is going to be working with the code who doesn't know the
language? Are you advocating that we change the way -core is written so that
its easy for those not familiar with the language? That we should eschew idiom
and use different approaches to facilitate that?
And what is the real problem with the global symbols? Are you suggesting that
people look through the library symbol table and choose methods/functions based
on what they find there? And that it is an issue for -core maintainers to address?
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.
All methods in objective-c 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).
Oh?
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.
Try:
$> strip
Just to be clear, I'm all for improving the code for reading, navigation and
maintenance. I simply disagree that this class wrapping is the right way to do
it. We can achieve all those goals without the class wrapper.
Regards,
Sheldon
_______________________________________________
Gnustep-dev mailing list
Gnustep-dev@gnu.org
http://lists.gnu.org/mailman/listinfo/gnustep-dev