8
\$\begingroup\$

Lately, I've been part of a team working on a utility library for iOS called Thundercats (on Github). We're about to start working on some changes and upgrades in preparation for a 2.0 release, so we figured it might be a good time to get some of our code publicly reviewed.

The library is written in Objective-C and is set up to work with Cocoapods, but we want to make sure we the library works well from iOS 6.0 and up and with both Swift and Objective-C.

This code is from the UIView extension. It provides an easy means of doing some common recursive searching.

UIView+TCAdditions.h

#import <UIKit/UIKit.h>

typedef NS_ENUM(NSUInteger, TCSearchStrategy) {
    TCSearchStrategyBreadthFirst,
    TCSearchStrategyDepthFirst
};

@interface UIView (TCAdditions)

/**
 *  Calls resignFirstResponder on this view or its subviews (1 level down) if the view is the first responder.
 */
- (void)tc_findAndResignFirstResponder;

/**
 *  Returns a flattened subview hierarchy. All subviews within this view's subview hierarchy are returned.
 *
 *  @return An array of all the view's subviews in its subview hierarchy.
 */
- (NSArray *)tc_getAllSubviewsRecursively;

/**
 *  Returns the first subview it encounters that satisfies the condition block. This method uses a breadth first search strategy.
 *
 *  @param conditionBlock The block to apply to each subview.
 *
 *  @return The first subview encountered that satisfies the condition block.
 */
- (UIView *)tc_subviewThatSatisfiesBlock:(BOOL (^)(UIView *))conditionBlock;

/**
 *  Returns the first subview it encounters that satisfies the condition block. This method can perform a breadth first search strategy or a depth first search strategy.
 *
 *  @param searchStrategy The search strategy to use.
 *  @param conditionBlock The block to apply to each subview.
 *
 *  @return The first subview encountered that satisfies the condition block.
 */
- (UIView *)tc_findSubviewUsingSearchStrategy:(TCSearchStrategy)searchStrategy
                           thatSatisfiesBlock:(BOOL (^)(UIView *view))conditionBlock;

@end

UIView+TCAdditions.m

#import "UIView+TCAdditions.h"

@implementation UIView (TCAdditions)

- (void)tc_findAndResignFirstResponder
{
    if ([self isFirstResponder])
    {
        [self resignFirstResponder];
        return;
    }

    for (UIView *subview in [self subviews])
    {
        if ([subview isFirstResponder])
        {
            [subview resignFirstResponder];
            return;
        }
    }
}


- (NSArray *)tc_getAllSubviewsRecursively
{
    NSMutableArray *subviews = [NSMutableArray new];
    for (UIView *subview in self.subviews)
    {
        [subviews addObject:subview];
        [subviews addObjectsFromArray:[subview tc_getAllSubviewsRecursively]];
    }

    return subviews;
}


- (UIView *)tc_subviewThatSatisfiesBlock:(BOOL (^)(UIView *))conditionBlock
{
    return [self tc_findSubviewUsingSearchStrategy:TCSearchStrategyBreadthFirst
                                thatSatisfiesBlock:conditionBlock];
}


- (UIView *)tc_findSubviewUsingSearchStrategy:(TCSearchStrategy)searchStrategy
                           thatSatisfiesBlock:(BOOL (^)(UIView *))conditionBlock
{
    switch (searchStrategy)
    {
        case TCSearchStrategyBreadthFirst:
            return [self tc_breadthFirstSubviewThatSatisfiesBlock:conditionBlock];

        case TCSearchStrategyDepthFirst:
            return [self tc_depthFirstSubviewThatSatisfiesBlock:conditionBlock];
    }
}


- (UIView *)tc_depthFirstSubviewThatSatisfiesBlock:(BOOL (^)(UIView *))conditionBlock
{
    for (UIView *subview in self.subviews)
    {
        if (conditionBlock(subview))
        {
            return subview;
        }
        else
        {
            UIView *view = [subview tc_depthFirstSubviewThatSatisfiesBlock:conditionBlock];

            if (view)
            {
                return view;
            }
        }
    }

    return nil;
}


- (UIView *)tc_breadthFirstSubviewThatSatisfiesBlock:(BOOL (^)(UIView *))conditionBlock
{
    NSMutableArray *nextViewsToCheck = [self.subviews mutableCopy];

    NSInteger i = 0;
    while (i < nextViewsToCheck.count)
    {
        UIView *view = nextViewsToCheck[i];
        if (conditionBlock(view))
        {
            return view;
        }
        else
        {
            i++;
            [nextViewsToCheck addObjectsFromArray:view.subviews];
        }
    }

    return nil;
}


@end

Any comments improving the readability, efficiency, or just making the code more idiomatic Objective-C would all be more than welcome.

\$\endgroup\$
7
\$\begingroup\$

I have only some small suggestions. The

- (NSArray *)tc_getAllSubviewsRecursively

method creates a (temporary) mutable array for each subview in the hierarchy. It might be more effective to create only a single array and passing this array around to append subviews recursively:

- (NSArray *)tc_getAllSubviewsRecursively {
    NSMutableArray *subviews = [NSMutableArray new];
    [self tc_addSubviewsRecursivelyTo:subviews];
    return subviews;
}

// Private helper method:
- (void)tc_addSubviewsRecursivelyTo:(NSMutableArray *)subviews {
    for (UIView *subview in self.subviews) {
        [subviews addObject:subview];
        [subview tc_addSubviewsRecursivelyTo:subviews];
    }
}

If you decide to use Xcode 7 and the latest OS X SDK at some point then you can use the Objective-C "Lightweight Generics" to specify that the returned array contains UIView objects:

- (NSArray<UIView *> *)tc_getAllSubviewsRecursively;

so that this causes a compiler warning:

NSString *foo = [self.view tc_getAllSubviewsRecursively][0];
// Incompatible pointer types initializing 'NSString *' with an expression of type 'UIView *'

The

- (void)tc_findAndResignFirstResponder;

method searches only one level deep for the first responder. You could implement it using the recursive search as

- (void)tc_findAndResignFirstResponder {
    if ([self isFirstResponder]) {
        [self resignFirstResponder];
        return;
    }

    UIView *subview = [self tc_subviewThatSatisfiesBlock:^BOOL(UIView *view) {
        return [view isFirstResponder];
    }];
    [subview resignFirstResponder];
}

where the final [subview resignFirstResponder] does nothing if the search return nil.

This would become even more simpler if you change the search methods to include the receiver itself in the search instead of starting the search at its subviews.


In both tc_depthFirstSubviewThatSatisfiesBlock: and tc_breadthFirstSubviewThatSatisfiesBlock the else is not necessary because the method returns in the if case. This saves one indentation level:

- (UIView *)tc_depthFirstSubviewThatSatisfiesBlock:(BOOL (^)(UIView *))conditionBlock
{
    for (UIView *subview in self.subviews) {
        if (conditionBlock(subview)) {
            return subview;
        }
        UIView *view = [subview tc_depthFirstSubviewThatSatisfiesBlock:conditionBlock];
        if (view) {
            return view;
        }
    }
    return nil;
}
\$\endgroup\$
  • \$\begingroup\$ The framework automatically dispatches nil-targeted action methods to the first responder, so an alternative to rummaging around the view hierarchy would be to simply send the following message: [[UIApplication sharedApplication] sendAction:@selector(resignFirstResponder) to:nil from:nil forEvent:nil]; \$\endgroup\$ – jlehr Sep 2 '15 at 15:54
2
\$\begingroup\$

I have three stylistic suggestions.

Regarding your method names: ...SubviewSatisfyingCondition: sounds more Cocoa to my ears than your wording; especially the pronoun "That" seems unusual/superfluous. Also, methods beginning with "get" should return values indirectly; thus tc_getAllSubviewsRecursively should just be tc_allSubviews (I'm not sure it's important to the caller that the method is recursive).

Second, it's hard to see what calling tc_depthFirstSubviewThatSatisfiesBlock: would mean with a nil Block, but right now that will cause a crash. I'd suggest that you make the requirement explicit and do at least one of:

  • Document that the Block must not be nil
  • Assert in the method that the Block is not nil

With the latter option, if for some reason a user of your library thinks that it makes sense to pass a nil Block, they will have an easier time figuring out what's going on if execution dies with a useful error message rather than an EXC_BAD_ACCESS.

A (probably undesirable) alternative would be to check for a nil Block and simply return the first subview. Regardless, something should be done about the nil case.

The third -- fairly minor -- point is that I would probably prefer, as a user of your library, to see a typedef BOOL (^TCSubviewPredicate)(UIView *); (or whatever name you like) in the header, and TCSubviewPredicate as the argument type for those search methods. Xcode will expand the type when I make the method call, but should I need to keep one of these Blocks around in a variable, its purpose is more explicit (and it's easier to type out).

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.