Hello David, thanks for the remarks. I've updated the patch and uploaded it to the review board. It takes care of most of what you mentioned, except for using -value: to obtain the return value of a closure. See below.
Cheers, Niels On Mon, Jun 22, 2009 at 12:51:13PM +0100, David Chisnall wrote: > When you are creating ranges, use NSMakeRange(). Done. Somehow, I didn't even notice that it existed. > Please don't put comments on the same line as braces; put them either > inside or outside the scope of the block on their own line. Noted. > I'm not sure that your loop sending conformsToProtocol: will work. > This will return YES if the class or its superclass conforms to the > protocol. I think you want to go up the inheritance chain looking for > the first class that doesn't conform to the protocol, and then use its > first direct subclass, but I can't really work out what you are trying > to achieve there. Please add comments explaining the 'why'; I can > tell the 'what' from the code. It does (or at least: is meant to) work the way you described. I have added some comments that hopefully explain what is going on within those messy loops and conditionals. > In initialisers, don't forget to test whether the call to super > returns nil. We also prefer guard clauses like this to return at the > start, rather than having the success case in a conditional body. Done as well. > Your ETCollectionMutationHOMProxy initWithCollection: method does > nothing, so please remove it. I removed the whole class since it's really of no use anymore. > You can eliminate the clang dependency with blocks by sending a - > value: message to blocks. This means that it will work with Smalltalk > blocks as well as Objective-C blocks (remember, blocks are objects > too). This will let you compile the framework code with GCC but code > using it with clang. I wasn't aware that the LanguageKit runtime was handling Objective-C blocks as well. But I'm not sure how to move to using BlockClosure objects instead of the ObjC2 block notation. I'm seeing two problems: * LanguageKit depends on EtoileFoundation, so without creating a circular dependency, the higher-order messaging code could not live there anymore. * for -filterWithBlock: I'm as of yet expecting BOOLs, not objects, to be returned by the closure. All the -value: methods seem to have id as their return type. I would intend to work around that by require an NSValue/NSNumber to be returned from the closure and unbox it in the filter code. Would that work with the Smalltalk implementation? > For the versions that take blocks as arguments, there is no point > creating the intermediate proxy object; you can just do the map (or > whatever) operation in the map method. To eliminate the code > duplication from doing this, put the method body in an inline static > function at the top of the file and call it from the proxy and the > category. This one gave me a little headache ;-) I now refactored the code that does the heavy lifting into static inline functions in a separate source file, which all have ugly names and make the whole thing appear not even remotly as convenient as I would have wished. > Also, please put the type of the argument in the method > name, for example -mappedArrayWithBlock: (yes, I know there is already > a -map:, it's my fault, and it's wrong; it's just there for Smalltalk > familiarity). I updated all the method names to adhere the coding style. I hope none slipped through. > The easiest way of implementing something like a mixin is just to move > the methods that you want to add to multiple classes into a separate > file and #include it inside the @implementation of each class. The > mixins stuff in EtoileFoundation isn't really designed for use inside > a framework. That seems to work quite reasonably. _______________________________________________ Etoile-dev mailing list Etoile-dev@gna.org https://mail.gna.org/listinfo/etoile-dev
