Monday, May 30, 2016

Completion Handlers

Have a single completion callback.

A completion handler interface should pass both the results and any errors:

- (void)chewTheBolusWithCompletionHandler:
   (void (^)(id result, NSError *error))callback;

It’s tempting to have separate success and failure callbacks. After all, apps will be handling each separately, right? It’s tempting, but it's dangerous. Success and failure cases both require cleanup by the app. Separating the handlers means cleanup will likely be omitted by accident for one of the callbacks.

Completion code should be called exactly once.

It's far easier for clients to use a simple, predictable interface. When a call takes a completion handler, be it a block or a selector, the handler should be called exactly once.  Calling zero times, or more than once, leaves clients unable to easily clean up their state.

Propagate the underlying error to the callback.

The underlying error should be passed back explicitly, especially from any code below the topmost UI level. If necessary, synthesize an NSError. Only the UI layer will know how to present the failure; for example, “server timeout” and “cannot reach host” may or may not need to be presented to users. The UI layer can make an appropriate decision only with an error to examine.

Some NSFileManager APIs originally just returned BOOLs to indicate success or failure, but those needed to be deprecated and replaced with versions that pass back NSErrors. Application code really does need to know the error that occurred.

Indicate failure only through the callback, not through a return value.

Methods that take a callback pointer should not also return a value. Rather, if they can fail, they should always call the callback, so the client only needs to handle failures in one place, not two.

Call back in a consistent manner for success and failure.

Most code succeeds often and fails rarely (or vice versa.) Since callbacks are typically tested more in one case than the other, each method should strive to always call back on a consistent thread or queue.

For example, if a method would succeed asynchronously and then call back on the main thread, it should be written so it also fails and calls back with an error asynchronously on the main thread. Consistency avoids bugs due to some unexpected, untested app state during the callback. A dispatch_async is fast, so the overhead is negligible.

 - (void)makeBrowniesWithCompletion:(void (^)(id brownies,  
                   NSError *error))callback {  
  if (!_hasIngredients) {  
   // We can’t make brownies. Call the callback with an error on the  
   // same queue as we would use if baking was successful.  
   dispatch_async(dispatch_get_main_queue(), ^{  
    NSError *ingredientError =   
     [NSError errorWithDomain:kMyAppDomain  
               code:kMyAppIngredientErrorCode  
             userInfo:nil];  
    callback(nil, ingredientError);  
   });  
   return;  
  }  
   
  [self preheatOvenAsyncWithCompletion:^{  
   // Preheating calls back on the main queue.  
   NSError *bakeError;  
   id brownies = [self bakeIngredientsWithError:&bakeError];  
   callback(brownies, bakeError);  
  }];  
 }  
 - (void)makeBrowniesWithCompletion:(void (^)(id brownies,  
                   NSError *error))callback {  
  if (!_hasIngredients) {  
   // We can’t make brownies. Call the callback with an error on the  
   // same queue as we would use if baking was successful.  
   dispatch_async(dispatch_get_main_queue(), ^{  
    NSError *ingredientError =   
     [NSError errorWithDomain:kMyAppDomain  
               code:kMyAppIngredientErrorCode  
             userInfo:nil];  
    callback(nil, ingredientError);  
   });  
   return;  
  }  
   
  [self preheatOvenAsyncWithCompletion:^{  
   // Preheating calls back on the main queue.  
   NSError *bakeError;  
   id brownies = [self bakeIngredientsWithError:&bakeError];  
   callback(brownies, bakeError);  
  }];  
 }  

Prevent accidental repeat callbacks.

It's safest if a callback pointer does not need to be stored in an ivar or a static variable. Block retain cycles won't happen when block pointers are in local variables. Try to structure your code to hold callback pointers just in local variables, and be especially wary of ever using a global or static variable for a block.

But if a callback pointer is stored, then remove the stored value before invoking the callback.

 - (void)invokeCallbackWithResult:(id)result  
               error:(NSError *)error {  
  dispatch_async(dispatch_get_main_queue(), ^{  
   void (^callback)(id result, NSError *error) = self.callback;  
   self.callback = nil;  
   if (callback) {  
    callback(result, error);  
   }  
  });  
 }  

Clearing the ivar or static before invoking the callback guarantees that nothing the callback does can lead to a repeat of the completion handler call.

Note too that in this sample, the callback is grabbed from the ivar inside the same block in which the callback block is called. This avoids creating an opportunity during the dispatch_async delay for the client code to cancel the operation and clear the callback property.

A window of opportunity during the dispatch_async might lead to the callback being invoked despite the operation having just been canceled if the callback pointer was retained in a local variable before the dispatch_async block. This may be appropriate if cancel shouldn’t stop the callback, though most often, callbacks aren’t desirable after an operation has been canceled.

Document, document, document.

Comments for the class, or for each method in the class, should describe the thread or queue expectations for callers of the methods, and the thread or queue used when invoking the method’s callback. Your future self will thank you for writing it down.

2 comments:

  1. I think makeBrowniesWithCompletion has a bug - it continues on with preheating the oven after reporting the lack of ingredients.

    ReplyDelete
  2. Nice catch; thank you for pointing it out. I've added the missing return statement.

    ReplyDelete