Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm using NSInvocation as a choke point in my app to help manage threading and reduce the amount of redundant code. The purpose of this class is to pass arguments from the view controllers to the API project on a background thread and return success/error on the main thread. I also have a log callback that logs API events in the background.

Here's my implementation file: (setArguments isn't being used and probably will be removed)

@implementation NSInvocation (Worker)
+(NSInvocation*)invocationWithSelector:(SEL)selector {
    return [NSInvocation invocationWithTarget:[WAAPI class] selector:selector arguments:nil];
}
+(NSInvocation*)invocationWithTarget:(id)target selector:(SEL)selector arguments:(NSArray*)arguments {
    NSMethodSignature *sig = [target methodSignatureForSelector:selector];
    NSInvocation *invocation = [NSInvocation invocationWithMethodSignature:sig];
    [invocation retainArguments];
    [invocation setTarget:target];
    [invocation setSelector:selector];
    if(arguments) {
        [invocation setArguments:arguments];
    }

    return invocation;
}

//SET ARGUMENTS
//===================================================================================================
-(void)setArguments:(NSArray*)arguments {
    //An integer (i in the below code) specifying the index of the argument. Indices 0 and 1 indicate the hidden arguments self and _cmd, respectively;
    //you should set these values directly with the setTarget: and setSelector: methods. Use indices 2 and greater for the arguments normally passed in a message.
#warning screwed up for blocks and probably primitive types
    for(int i = 0; i < arguments.count; i++) {
        id argument = [arguments objectAtIndex:i+2];
        if([argument isKindOfClass:NSClassFromString(@"NSBlock")]) {
            [self setBlockArgument:argument index:i];
        } else {
            [self setArgument:&argument atIndex:i];
        }
    }
}
-(void)setBlockArgument:(id)blockArgument index:(NSInteger)index {

}
-(void)setAsWeakArgument:(id)argument index:(NSInteger)index {
    __weak id weakArg = argument;
    [self setArgument:&weakArg atIndex:index];
}

//MAKE THE CALL
//===================================================================================================
-(NSOperation*)invokeAPICall {
    @autoreleasepool {
        [WAWorker background:^{
            [self invoke];
        }];
        return [self apiReturnValue];
    }
}

//RETURN VALUE
//===================================================================================================
-(NSOperation*)apiReturnValue {
    __unsafe_unretained NSOperation *result;
    [self getReturnValue:&result];
    return result;
}

//SUCCESS
//===================================================================================================
-(void)setSuccess:(void(^)(id))success index:(NSInteger)index {
    [self setSuccess:success title:nil message:nil index:index];
}
-(void)setSuccess:(void(^)(id))success title:(NSString*)title message:(NSString*)message index:(NSInteger)index {
    void(^successBlock)(id) = ^(id s){
        [WAWorker main:^{
            if(title && message) {
                [WANotificationManager showSuccessWithTitle:title message:message];
            }
            if(success) success(s);
        }];
    };

    [self setArgument:&successBlock atIndex:index];
}

//PROGRESS
//===================================================================================================
-(void)setProgress:(void(^)(double))progress index:(NSInteger)index {
    void(^progressBlock)(double) = ^(double p){
        [WAWorker main:^{
            if(progress) progress(p);
        }];
    };

    [self setArgument:&progressBlock atIndex:index];
}

//ERROR
//===================================================================================================
-(void)setError:(void(^)(NSError*))error index:(NSInteger)index {
    [self setError:error message:nil index:index];
}
-(void)setError:(void(^)(NSError*))error message:(NSString*)message index:(NSInteger)index {
    void(^errorBlock)(NSError*) = ^(NSError *e){
        [WAWorker main:^{
            if(message) {
                [WAErrorHandler processError:e defaultText:message additionalText:[NSString stringWithFormat:@"%s",sel_getName(self.selector)]];
            }
            if (error) error(e);
        }];
    };

    [self setArgument:&errorBlock atIndex:index];
}

//LOG
//===================================================================================================
-(void)setLogIndex:(NSInteger)index {
    void(^logBlock)(NSString*, LogType) = ^(NSString *log, LogType logType){
        [WAConsoleManager log:log type:logType];
    };

    [self setArgument:&logBlock atIndex:index];
}
@end

Here's a couple examples using it:

-(void)updateProfileWithSuccess:(void(^)(void))success error:(void(^)(NSError*))error {
    NSInvocation *invocation = [NSInvocation invocationWithSelector:@selector(profileUpdate:deleted:success:error:log:)];

    void(^successBlock)(id) = ^(WAContact *s) {
        self.dataContact = s;
        [WAWorker main:^{
            [WANotificationManager showSuccessWithTitle:@"Success" message:@"Your information has been updated"];
            if(success) success();
        }];
    };

    WAContact *contact = self.dataContact;
    NSArray *deleted = self.dataDeleted;
    [invocation setArgument:&contact atIndex:2];
    [invocation setArgument:&deleted atIndex:3];
    [invocation setArgument:&successBlock atIndex:4];
    [invocation setError:error message:@"Failed to update your information" index:5];
    [invocation setLogIndex:6];
    [invocation invokeAPICall];
}

-(void)messageDetails:(NSString*)messageID success:(void (^)(void))success error:(void (^)(NSError*))error {
    NSInvocation *invocation = [NSInvocation invocationWithSelector:@selector(messageDetails:success:error:log:)];
    void(^successBlock)(id) = ^(id s) {
        self.dataMessage = s;
        [WAWorker main:^{
            if(success) success();
        }];
    };

    [invocation setArgument:&messageID atIndex:2];
    [invocation setArgument:&successBlock atIndex:3];
    [invocation setError:error index:4];
    [invocation setLogIndex:5];
    [invocation invokeAPICall];
}

Questions:

  1. Am I creating retain loops by passing self in any of the blocks?

  2. Is this possible without using retainArguments: in the NSInvocation category?

  3. Are there any other issues with this implementation?

share|improve this question

1 Answer 1

This is one of my weakspots, but if you are ever concerned about a retain loop in a block, create a weak variable:

__weak typeof(self) weakSelf = self;

Now just use weakSelf anywhere you'd use self in that block, and there's no chance that the block will create a retain cycle.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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