A couple of reactions:
Before you tackle the retry logic, you should probably move your call to
[self completeOperation]
to inside the completion block of theNSURLSessionDataTask
orsendAsynchronousRequest
. Your current operation class is completing prematurely (and therefore would not honor dependencies and your network operation queue's intendedmaxConcurrentOperationCount
).The retry logic seems unremarkable. Perhaps something like:
- (void)main { NSURLRequest *request = [self createRequest]; // maybe move the request creation stuff into its own method [self tryRequest:request currentDelay:1.0]; } - (void)tryRequest:(NSURLRequest *)request currentDelay:(NSTimeInterval)delay { [NSURLConnection sendAsynchronousRequest:request queue:[self networkOperationCompletionQueue] completionHandler:^(NSURLResponse *response, NSData *data, NSError *connectionError) { BOOL success = NO; if (connectionError) { NSLog(@"%@ Error: %@", self.serviceName, connectionError.localizedDescription); } else { if ([response isKindOfClass:[NSHTTPURLResponse class]]) { NSInteger statusCode = [(NSHTTPURLResponse *)response statusCode]; if (statusCode == 200) { // parse XML response here; if successful, set `success` to `YES` } } } if (success) { [self completeOperation]; } else { dispatch_time_t popTime = dispatch_time(DISPATCH_TIME_NOW, (int64_t)(delay * NSEC_PER_SEC)); dispatch_after(popTime, dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^(void){ NSTimeInterval nextDelay = [self nextDelayFromCurrentDelay:delay]; [self tryRequest:request currentDelay:nextDelay]; }); } }]; }
Personally, I'm wary about this entire endeavor. It strikes me that you should be employing logic conditional upon the type of error. Notably, if the error is a failure resulting from lacking of internet connectivity, you should use Reachability to determine connectivity and respond to notifications to retry automatically when connectivity is restored, not simply retrying at prescribed mathematical progression of retry intervals.
Other than network connectivity (which is better addressed with Reachability), I'm unclear as to what other network failures warrant a retry logic.
Some unrelated observations:
Note, I eliminated the
dispatch_async
of the issuing of the request inmain
to a background queue because you're using asynchronous methods already (and even if you weren't, you've presumably added this operation to a background queue, anyway).I've also removed the
try
/catch
logic because, unlike other languages/platforms, exception handling is not the preferred method of handling runtime errors. Typically runtime errors in Cocoa are handled viaNSError
. In Cocoa, exceptions are generally used solely to handle programmer errors, but not to handle the runtime errors that a user would encounter. See Apple's discussion Dealing with Errors in the Programming with Objective-C guide.You can get rid of your manually implemented
isExecuting
andisFinished
getter methods if you just define the appropriate getter method for your properties during their respective declarations:@property (nonatomic, readwrite, getter=isExecuting) BOOL executing; @property (nonatomic, readwrite, getter=isFinished) BOOL finished;
You might, though, want to write your own
setExecuting
andsetFinished
setter methods, which do the notification for you, if you want, e.g.:@synthesize finished = _finished; @synthesize executing = _executing; - (void)setExecuting:(BOOL)executing { [self willChangeValueForKey:kExecutingKey]; _executing = executing; [self didChangeValueForKey:kExecutingKey]; } - (void)setFinished:(BOOL)finished { [self willChangeValueForKey:kFinishedKey]; _finished = finished; [self didChangeValueForKey:kFinishedKey]; }
Then, when you use the setter it will do the notifications for you, and you can remove the
willChangeValueForKey
anddidChangeValueForKey
that you have scattered about your code.Also, I don't think you need to implement
isCancelled
method (as that's already implemented for you). But you really should override acancel
method which calls itssuper
implementation, but also cancels your network request and completes your operation. Or, instead of implementingcancel
method, you could move to thedelegate
based rendition of the network requests but make sure you check for[self isCancelled]
inside thedidReceiveData
method.And
isCompleted
strikes me as redundant withisFinished
. It seems like you could entirely eliminatecompleted
property andisCompleted
method.You're probably unnecessarily duplicating the amount of network code by supporting both
NSURLSession
andNSURLConnection
. You can do that if you really want, but they assure us thatNSURLConnection
is still supported, so it strikes me as unnecessary (unless you wanted to enjoy someNSURLSession
specific features for iOS 7+ devices, which you're not currently doing). Do whatever you want, but personally, I'm usingNSURLConnection
where I need to support earlier iOS versions, andNSURLSession
where I don't, but I wouldn't be inclined to implement both unless there was some compelling business requirement to do so.