There are a few of issues here:
The geocodeAddressString imposes a few limitations, as outlined in the documentation:
This method submits the specified location data to the geocoding server asynchronously and returns. Your completion handler block will be executed on the main thread. After initiating a forward-geocoding request, do not attempt to initiate another forward- or reverse-geocoding request.
Geocoding requests are rate-limited for each app, so making too many requests in a short period of time may cause some of the requests to fail. When the maximum rate is exceeded, the geocoder passes an error object with the value kCLErrorNetwork to your completion handler.
Several key observations here:
This runs asynchronously (so you cannot call geocodeAddressString and use its results immediately afterwards). You have do invoke the work contingent on the geocoding inside the completion block.
You should not be starting the next geocode request until the prior one completes.
This means that you have to geocode the first postal code, let it complete asynchronously (i.e. later), geocode the next one, let it complete, etc., and only then do your sort and reload the table. A simple for loop is not an appropriate way to do this. You can either write a method that does a single geocode and invokes the next geocode in the completion block, or you can use NSOperation subclass as I have below.
I would advise storing the distance as a NSNumber. In MVC, the one decimal place string representation is a "view" behavior, and should probably not be part of the "model".
The advantage of this is that when you want to sort the objects, you can simply invoke the compare method for the NSNumber. For example, if salesPersonnel was a NSMutableArray of objects which each SalesPerson object has the NSNumber property called distance, you could then do:
[self.salesPersonnel sortUsingComparator:^NSComparisonResult(SalesPerson *obj1, SalesPerson *obj2) {
return [obj1.distance compare:obj2.distance];
}];
I wasn't sure if your sales entries per actual sales transactions or sales personnel, so I apologize if I misinterpreted the object types, but hopefully this illustrates the idea.
You can do this any way you want, but for me, when I want to run a number of asynchronous tasks, but do so sequentially, I gravitate to concurrent NSOperation subclass which I'll add to a serial NSOperationQueue.
NSError *error;
NSArray *addressEntries = [NSJSONSerialization JSONObjectWithData:data options:0 error:&error];
NSAssert(addressEntries, @"unable to parse: %@", error);
NSOperationQueue *queue = [[NSOperationQueue alloc] init];
queue.maxConcurrentOperationCount = 1;
self.salesPersonnel = [NSMutableArray array];
// define sort operation that will be called when all of the geocode attempts are done
NSOperation *sortAndReloadTableOperation = [NSBlockOperation blockOperationWithBlock:^{
[self.salesPersonnel sortUsingComparator:^NSComparisonResult(SalesPerson *obj1, SalesPerson *obj2) {
return [obj1.distance compare:obj2.distance];
}];
[self.tableView reloadData];
}];
// create the geocode operations
for (NSDictionary *addressEntry in addressEntries) {
SalesPerson *salesPerson = [[SalesPerson alloc] initWithSalesId:addressEntry[@"id"]
name:addressEntry[@"name"]
address:addressEntry[@"address"]
postalCode:addressEntry[@"postcode"]];
[self.salesPersonnel addObject:salesPerson];
NSOperation *geocodeOperation = [[GeocodeOperation alloc] initWithPostalCode:salesPerson.postalCode completionHandler:^(NSArray *placemarks, NSError *error) {
CLPlacemark *placemark = [placemarks firstObject];
CLLocation *location = placemark.location;
CLLocationDistance meters = [location distanceFromLocation:self.currentLocation];
salesPerson.distance = @(meters / 1609.344);
}];
[sortAndReloadTableOperation addDependency:geocodeOperation]; // note, the final sort is dependent upon this finishing
[queue addOperation:geocodeOperation]; // go ahead and queue up the operation
}
// now we can queue the sort and reload operation, which won't start until the geocode operations are done
[[NSOperationQueue mainQueue] addOperation:sortAndReloadTableOperation];
And the GeocodeOperation is a basic concurrent NSOperation subclass:
// GeocodeOperation.h
#import <Foundation/Foundation.h>
typedef void(^GeocodeCompletionHandler)(NSArray *placemarks, NSError *error);
@interface GeocodeOperation : NSOperation
@property (nonatomic, copy) GeocodeCompletionHandler geocodeCompletionHandler;
- (instancetype)initWithPostalCode:(NSString *)postalCode completionHandler:(GeocodeCompletionHandler)geocodeCompletionHandler;
@end
and the implementation (note, the main method is the only interesting bit here ... all the rest is routine concurrent NSOperation subclass code; personally, I move all of the concurrent NSOperation stuff into a base class, which cleans up this GeocodeOperation code, but I didn't want to confuse this further, so I've kept this simple):
// GeocodeOperation.m
#import "GeocodeOperation.h"
@import CoreLocation;
@interface GeocodeOperation ()
@property (nonatomic, readwrite, getter = isFinished) BOOL finished;
@property (nonatomic, readwrite, getter = isExecuting) BOOL executing;
@property (nonatomic, copy) NSString *postalCode;
@end
@implementation GeocodeOperation
@synthesize finished = _finished;
@synthesize executing = _executing;
- (CLGeocoder *)sharedGeocoder
{
static CLGeocoder *geocoder = nil;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{
geocoder = [[CLGeocoder alloc]init];
});
return geocoder;
}
- (instancetype)initWithPostalCode:(NSString *)postalCode completionHandler:(GeocodeCompletionHandler)geocodeCompletionHandler
{
self = [super init];
if (self) {
_postalCode = [postalCode copy];
_geocodeCompletionHandler = geocodeCompletionHandler;
}
return self;
}
- (void)main
{
[[self sharedGeocoder] geocodeAddressString:self.postalCode completionHandler:^(NSArray *placemarks, NSError *error) {
if (self.geocodeCompletionHandler) {
self.geocodeCompletionHandler(placemarks, error);
}
[self completeOperation];
}];
}
#pragma mark - NSOperation methods
- (void)start
{
if ([self isCancelled]) {
self.finished = YES;
return;
}
self.executing = YES;
[self main];
}
- (void)completeOperation
{
self.executing = NO;
self.finished = YES;
}
- (BOOL)isConcurrent
{
return YES;
}
- (void)setExecuting:(BOOL)executing
{
if (_executing != executing) {
[self willChangeValueForKey:@"isExecuting"];
_executing = executing;
[self didChangeValueForKey:@"isExecuting"];
}
}
- (void)setFinished:(BOOL)finished
{
if (_finished != finished) {
[self willChangeValueForKey:@"isFinished"];
_finished = finished;
[self didChangeValueForKey:@"isFinished"];
}
}
@end
