Asynchronous programming in Objective-C: I feel like there is repetitive code here and I don't know what I can do about it

Question

I have been working with async programming for a while now and I think I understand the concepts, but there is a certain scenario I feel like I am not getting. Check out the code:

-(void)someMethod:completionHandler:(void (^)(int result))handler
{
    [anotherObject asyncMethod1Success:^(NSDictionary *dict)
    {
        if ([dict[@"someKey"] isEqualToString:kString1])
        {
            // some code

            if (handler)
            {
                handler(1);
            }
        }
        else if ([dict[@"someKey"] isEqualToString:kString2])
        {
            // some different code

            if (handler)
            {
                handler(1);
            }
        }
        else if ([dict[@"someKey"] isEqualToString:kString3])
        {
            // even different code

            [anotherObject asyncMethod2Success:^(NSDictionary *dict)
            {
                if (handler)
                {
                    handler(1);
                }
            }
            failure:^(NSError *error)
            {
                if (handler)
                {
                    handler(2);
                }
            }];
        }
    }
    failure:^(NSError *error)
    {
        if (handler)
        {
            handler(2);
        }
    }];
}

Basically, handler needs to get called once there is either an error or both async operations return successfully. I feel like there is repetitive code here and I don't know what I can do about it. I can't call handler() unconditionally in asyncMethod1's success block because case #3 needs it's async method to succeed or fail for it to be called.

Can anyone suggest a pattern to help here? I feel the most uncomfortable when working with nested async operations like this one.

Answer 1

I think your repetitive example is not only related to asynchronous aspect of that method. I would focus on removing duplicates from if-else branches, too.

Tips to reduce the code:

• Don't create methods taking success and failure block. Use one block to report both cases. (Yes, sometimes you can't just change existing methods.)
• Check for block being nil only once and use some default empty block in that case.
• Extract code that is common to all if-else branches outside of them.
• If possible, avoid doing if-else at all. Sometimes it's easier to use some kind of mapping with NSDictionary or other approach.

Here is the code with applied tips:

- (void)someMethodWithCompletionHandler:(void (^)(int result))handler {

    if ( ! handler) handler = ^(int result) {}; // Use empty block, so it's never nil.

    [anotherObject asyncMethod1Success:^(NSDictionary *dict) {

        NSString *someString = dict[@"someKey"];
        int result = 0; // Default

        if ([someString isEqualToString:kString1]) {
            // some code
            result = 1;
        }
        else if ([someString isEqualToString:kString2]) {
            // some different code
            result = 1;
        }
        // ... maybe even more branches
        else if ([someString isEqualToString:kString3]) {
            // even different code

            [anotherObject asyncMethod2Success:^(NSDictionary *dict) {
                handler(1);
            }
            failure:^(NSError *error) {
                handler(2);
            }];
        }

        if (result) handler(result);
    }
    failure:^(NSError *error) {
        handler(2);
    }];
}

I'm not sure how many result codes do you have. If you only indicates success/failure, there may be easier approach.

Answer 2

How about using blocks?

-(void)someMethod:completionHandler:(void (^)(int result))handler
{
    void (^safeHandler)(int) = ^(int value) {
        if (handler)
        {
            handler(value);
        }
    };

    [anotherObject asyncMethod1Success:^(NSDictionary *dict)
    {
        if ([dict[@"someKey"] isEqualToString:kString1])
        {
            // some code

            safeHandler(1);
        }
        else if ([dict[@"someKey"] isEqualToString:kString2])
        {
            // some different code

            safeHandler(1);
        }
        else if ([dict[@"someKey"] isEqualToString:kString3])
        {
            // even different code

            [anotherObject asyncMethod2Success:^(NSDictionary *dict)
            {
                safeHandler(1);
            }
            failure:^(NSError *error)
            {
                safeHandler(2);
            }];
        }
    }
    failure:^(NSError *error)
    {
        safeHandler(2);
    }];
}

Answer 3

EDIT Here's an example that's closer to your original code, maybe you like this arrangement?

enum CompletionResult { Fail, Success };

@interface Test : NSObject
@property ( nonatomic, readonly ) dispatch_queue_t q ;
@end

@implementation Test

-(dispatch_queue_t)q
{
    return dispatch_get_main_queue() ;
}

-(void)method1WithSuccessBlock:(void(^)(NSDictionary * dict))successBlock failureBlock:(void(^)(void))failBlock
{
    // long running method 1
    // return YES/NO on success/failure
}

-(void)method2WithSuccessBlock:(void(^)(NSDictionary * dict))successBlock failureBlock:(void(^)(void))failBlock
{
    // long running method 2
    // return YES/NO on success/failure
}

-(void)test:(void(^)(enum CompletionResult))block
{
    __block BOOL performAsync2 = NO ;
    void (^successBlock)(NSDictionary * dict) = ^(NSDictionary * dict){
        NSString * value = dict[@"someKey"] ;
        if ( [ value isEqualToString:A ] )
        {

        }
        else if ( [ value isEqualToString:B ])
        {

        }
        else if ( [ value isEqualToString:C ] )
        {
            performAsync2 = YES ;
        }

        if (  !performAsync2 && block )
        {
            block( Success ) ;
        }

    };
    void (^failBlock)() = ^{
        if ( block ) { block( Fail ) ; }
    };

    dispatch_sync( self.q, ^{ [ self method1WithSuccessBlock:successBlock failureBlock:failBlock ] } ) ;
    if ( performAsync2 )
    {
        dispatch_sync( self.q, ^{ [ self method2WithSuccessBlock:successBlock failureBlock:failBlock ] } ) ;
    }
}

@end

---

Could you do something like this?

@interface Test : NSObject
@property ( nonatomic, readonly ) dispatch_queue_t q ;
@end
@implementation Test

-(dispatch_queue_t)q
{
    return dispatch_get_main_queue() ;
}
-(BOOL)method1
{
    // long running method 1
    // return YES/NO on success/failure
}

-(BOOL)method2
{
    // long running method 2
    // return YES/NO on success/failure
}

-(void)test:(void(^)(enum CompletionResult))block
{
    __block BOOL didSucceed = NO ;
    dispatch_sync( self.q, ^{ didSucceed = [ self method1 ] ; }) ;
    if ( !didSucceed )
    {
        if ( block ) { block( Fail ) ; }
        return ;
    }
    dispatch_sync( self.q, ^{ didSucceed = [ self method2 ] ; }) ;
    if ( !didSucceed )
    {
        if ( block ) { block( Fail ) ; }
        return ;
    }

    if ( block ) { block( Success ) ; }
}

@end

You'd have to change how you call async1 and async2... but the main code is simple to follow. Make sure the queue you use runs in the background of course.

Answer 4

The biggest change I made was to define a block variable that took care of the multiple checks for whether or not handler existed. To alleviate your uneasiness with nested asynchronous methods I just defined a new method and called it. The rest of the changes are just formatting. In the end, I think this still provides the same functionality and is a little more compact and easier to follow.

I wasn't sure if asyncMethod1 and 2 were objects or not, so I just assumed they are methods defined within the same class and inserted self.

- (void)callAsyncMethod2WithHandler:(void (^)(int result))handler {
    [self asyncMethod2Success:^(NSDictionary *dict) {
        handler(1);
    } failure:^(NSError *error) {
        handler(2);
    }];
}

- (void)someMethod:(void (^)(int result))handler {
    void (^safeHandler)(int) = ^void (int theResult) {
        if (handler) handler(theResult);
    };

    [self asyncMethod1Success:^(NSDictionary *dict) {
        NSString *someValue = dict[@"someKey"];
        if ([someValue isEqualToString:kString1]) {
            // some code
            safeHandler(1);
        } else if ([someValue isEqualToString:kString2]) {
            safeHandler(1);
        } else if ([someValue isEqualToString:kString3]) {
            [self callAsyncMethod2WithHandler:safeHandler];
        }
    } failure:^(NSError *error) {
        safeHandler(2);
    }];
}

Answer 5

It seems like a contrived example, but if you don't mind to utilize a small third party library, your code can be mimicked as follows:

handler_block_t handler = ...;

RXPromise* p0 = someMethod();

p0.then(^id(id dict) {
    if ([dict[@"someKey"] isEqualToString:kString1]) {
        // some code
        return @"OK";
    }
    else if ([dict[@"someKey"] isEqualToString:kString2]) {
        return @"OK";
    }
    else if ([dict[@"someKey"] isEqualToString:kString3]) {
        return asyncMethod2();
    } else {
        return nil; // not handled
    }
}, nil).then(^id(id result /* dict or @"OK" or nil */) {
    // parameter result is either @"OK", nil (not handled), or asyncMethod2's eventual result value:
    if (result != nil) {
        handler(1);
    }
    return nil;
}, ^id(NSError* error) {
    handler(2);
    return nil;
});

Of course, if you would utilize Promises (there are a few libraries in Objective-C which implement it) , you likely would restructure the code in order to become more comprehensible.

I'm the author of RXPromise, which is an Objective-C library which implements the concept of Promises according the Promises/A+ Specification.

Answer 6

This isn't really an answer, but it's a reformatting of your code to make it a little simpler. It may not work (depending on what the //some code is). First what I see in your code is that it does nothing if the handler is not valid (this is where the //some code changes my response. If you put a return into the only if/then case where you do not call handler(1) then you can call it at the end of the function.

-(void)someMethod:completionHandler:(void (^)(int result))handler
{
    if(handler)
    {
        [asyncMethod1 success:^(NSDictionary *dict)
        {
            NSString *test = dict[@"someKey"];
            if (test isEqualToString:kString1])
            {
                // some code
            }
            else if (test isEqualToString:kString2])
            {

            }
            else if (test isEqualToString:kString3])
            {
                [asyncMethod2 success:^(NSDictionary *dict)
                 {
                    handler(1);
                }
                failure:^(NSError *error)
                {
                    handler(2);
                }];
                return;
            }
            handler(1);
        }
        failure:^(NSError *error)
        {
            handler(2);
        }];
    }
}