youknowone / FoundationExtension

Foundation/Cocoa/UIKit extension kit. Reference document:
http://youknowone.github.io/FoundationExtension
Other
121 stars 22 forks source link

Prefix category methods on core Objective C classes. #6

Open tonyxiao opened 11 years ago

tonyxiao commented 11 years ago

Installed UI7Kit and thus FoundationExtension earlier but got a weird hard-to-debug crash. Took a while to figure out but finally tracked down the culprit to

@implementation NSMutableArray (Functional)

- (void)map:(NSAObjectUnaryOperator)mapper {
    NSUInteger count = self.count;
    for (NSUInteger i = 0; i < count; i++) {
        self[i] = mapper(self[i]);
    }
}

This category implementation overrode a previous category I have on NSMutableArray (though BlocksKit), which caused the crash at runtime. NSMutableArray is such a generic class and map: is such a generic method signature that it's almost guaranteed to run into method name clash problems for any projects of reasonable size and complexity, making FoundationExtension (and by extension UI7Kit) unusable :(. Could take an approach similar to MagicalRecord (https://github.com/magicalpanda/MagicalRecord/blob/develop/MagicalRecord/CoreData%2BMagicalRecord.h#L14), define a macro if you want the shorthand version offered by FoundationExtension, otherwise prefix all category methods on core Objective C classes with fe_ for Foundation Extension.

youknowone commented 11 years ago

First, I am sorry that my codes raised hard-to-debug problems. Yes - compile time issues are really hard to track.

This is not a main issue, but I really want to point out a thing makes me uncomfortable. Your suggestion is really unfair. I didn't overload existing methods. It is overloaded because you installed BlocksKit. So, I checked your issues to BlocksKit and I found no issue about this problem in BlocksKit. You found 2 third-party category extensions make name collision and decided to require renaming to unpopular one. (Remember you pointed out nothing special but 'generic class' and 'generic method signature' which can be adopt to every category extension) If you wanted to suggest that renaming only to this project, you should have suggested some better reasons only for this project - especially if the suggetion breaks all the convention of the codes. Name collision is a kind of unpredictable and easily possible problem of category extension and most of we all never choose ugly prefix regardless it is recommended or not.

About the rejection of patch, there is other issues. FoundationExtension is intending to have Foundation.framework-like method names.

  1. Any prefix breaks the naming policy.
  2. Your patch is only about NSAFunctional so it breaks naming consistency of library.

Because BlocksKit is one of the mainstream, I think your report is obvious bug for this project. But I hope to gain better suggestions for the method names which will not be used in other project and also easily can be regard as Apple convention, rather than prefixed methods.

Thanks for great report. I probably never found this issue without your report :)