On 14 Aug 2011, at 22:09, Fred Kiefer wrote: > On 14.08.2011 22:57, Richard Frith-Macdonald wrote: >> >> On 14 Aug 2011, at 21:20, Fred Kiefer wrote: >> >>> I just noticed that we have lots of reimplementation of NSString >>> methods in the sub classes in GSString.m. Some of these are >>> actually needed like implementing >>> -initWithBytesNoCopy:length:encoding:freeWhenDone:, many others >>> look plain wrong. Why would we duplicate code here, in some cases >>> even in a worse form? After the current code freeze on base, could >>> somebody please go through this file and clean out the duplicated >>> methods? >> >> Subclasses are *supposed* to reimplement the methods of a class >> cluster ... 1. the primitive methods have to be implemented for the >> classes to function at all 2. many/most other methods need to be >> implemented in terms of the concrete ivars actually used ... so we >> get decent performance. >> >> There shouldn't be any real duplication of code, but there should be >> a *lot* of methods re-implemented ... to access the 8 or 16 bit >> character data within the strings directly. These should *not* be >> 'cleaned out' since they are essential for decent performance. > > There seems to be a disagreement here, so lets look at one example. > GSPlaceholderString has this method: > > - (id) initWithCharacters: (const unichar*)chars > length: (NSUInteger)length > { > return [self initWithBytes: (const void*)chars > length: length * sizeof(unichar) > encoding: NSUnicodeStringEncoding]; > } > > NSString has the following corresponding method: > > - (id) initWithCharacters: (const unichar*)chars > length: (NSUInteger)length > { > return [self initWithBytes: chars > length: length * sizeof(unichar) > encoding: NSUnicodeStringEncoding]; > } > > I don't quite see how the only difference (the type cast) is "essential for > decent performance".
Yes, that does look like a case where we have an unnecessary method override ... but I had the (possibly mistaken) impression you were talking about a *lot* of code and 'worse' code. It would surprise me if there is much of that. > I do understand that what you wrote is the basic idea of what should go into > this file. I just have the impression that at the moment this isn't what is > in the file and I would like to see that corrected. I also know that this has > to be done very carefully as just using the implementation in NSString might > result in circular call sequences or very bad performance. Fine ... I agree with that (but don't have time to look at it right now). IMO the ideal would be if someone could: 1. identify any cases of duplicate/wrong methods 2. write tests for the testsuite to exercise those methods 3. remove the duplication and check that the tests still pass _______________________________________________ Gnustep-dev mailing list Gnustep-dev@gnu.org https://lists.gnu.org/mailman/listinfo/gnustep-dev