Sounds very shiny.
Nicolas: Can you fix the review board? Reading diffs is no fun...
A few things:
When you are creating ranges, use NSMakeRange(). This will expand to
the same thing as your code, but is more readable because the
declaration and initialisation go together, like this:
NSRange r = NSMakeRange(0,5);
Because you are only using the range once, it's cleaner to just use
NSMakeRange directly, like:
[NSIndexSet indexSetWithIndexesInRange: NSMakeRange(0,5)];
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.
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.
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, so
write things like this:
if (nil == (self = [super initWithSomeDesignatedInitialiser: wibble]))
{ return nil; }
Your ETCollectionMutationHOMProxy initWithCollection: method does
nothing, so please remove it.
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.
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. 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).
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.
Other than that, it looks good.
David
On 22 Jun 2009, at 11:47, Niels Grewe wrote:
> Hello all,
>
> I had some time to spare over the last week or so and put together
> some
> higher-order messaging stuff that seems to work suprisingly well. The
> attached patch represents the results of my work, implementing
> map, fold and filter for arbitrary collections, both with and without
> blocks. Please note:
>
> * While the code is quite generic (with some bits and pieces mostly
> handling the special requirements of NSDictionary and
> NS(Counted|Index)Set), I did tie it into the class hierarchy through
> categories on the specific ETCollection-adopting classes, meaning you
> get -mappedArray, -mappedDictionary, -mappedSet etc. where
> -mappedCollection would have been sufficient. This is somewhat clumsy
> and I think it should be possible to unify this by mixins/traits, but
> I have yet to grasp how to use them.
> * The method variants using blocks are only enabled when compiling
> with
> clang. They seem to work, but I could not do extensive testing
> because
> I'm still working out some kinks with clang. [0] But if you're not
> using clang, everything should still build and work fine.
> * In this implementation, fold works as suggested by David.
> * Some (naive) unit tests are included for the forwardInvocation-using
> method variants.
> * At least with the GNU runtime, you will have problems with
> ETGetSuperclass() unless you apply the patch I suggested on Thursday
> (or something similar).
>
> As always, I'm soliciting comments, suggestions and criticisms on
> this,
> especially since this is the first bigger chunk of code I'm putting
> out
> for the whole world to see...
>
> Cheers,
>
>
> Niels
> --
> [0] E.g.: When compiling with clang I always get a segfault when
> -[NSArray arrayWithObject:] is called.
> <EtoileFoundation
> +HOM.diff>_______________________________________________
> Etoile-dev mailing list
> [email protected]
> https://mail.gna.org/listinfo/etoile-dev
_______________________________________________
Etoile-dev mailing list
[email protected]
https://mail.gna.org/listinfo/etoile-dev
