Model to handle all the data/networking from Foursquare API in IOS

Question

I posted not too long ago, asking for tips/improvements I could use on my models. I have changed quite a lot, so I thought I'll give it another go.

My model is supposed to handle all the data I get from the Foursquare API:

venueService.h

typedef void (^TNVenueServiceCompletionBlock)(UIImage *image, NSError *error);

@class TNVenueImageData;

@interface TNVenueService : NSObject

@property (nonatomic) NSString *clientID;
@property (nonatomic) NSString *clientSecret;
@property (nonatomic) NSString *radius;

@property (nonatomic) NSArray *venueObject;
@property (nonatomic) NSArray *imageData;

+ (instancetype)venueService;

- (NSDictionary *)mainCategoryKeys;
- (NSArray *)mainCategories;
- (void)performVenueLocationRequest:(CLLocation *)location identifier:(NSString *)identifier;
- (void)performPhotoDetailsRequest:(NSString *)identifier;
- (void)downloadImage:(TNVenueImageData *)imageData completionBlock:(TNVenueServiceCompletionBlock)completion;

@end

venueService.m

@implementation TNVenueService

+ (instancetype)venueService
{
    TNVenueService *venueService = [[TNVenueService alloc] init];
    venueService.clientID = @"MY_CLIENT_ID";
    venueService.clientSecret = @"MY_CLIENT_SECRET";
    venueService.radius = @"2000"; //start-radius

    return venueService;
}


- (NSURLRequest *)URLRequestWithFormat:(NSString *)attributes, ...
{
    va_list arguments;

    va_start(arguments, attributes);
    NSString *urlPath = [[NSString alloc] initWithFormat:attributes arguments:arguments];
    va_end(arguments);
    NSURL *url = [NSURL URLWithString:urlPath];
    NSURLRequest *request = [NSURLRequest requestWithURL:url];

    return request;
}

- (void)performVenueLocationRequest:(CLLocation *)location identifier:(NSString *)identifier
{
    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    dispatch_async(queue, ^{
        NSError *error = nil;
        NSURLResponse *response = nil;

        NSData *data = [NSURLConnection sendSynchronousRequest:[self buildRequestForVenueLocation:location identifier:identifier]
                                             returningResponse:&response error:&error];

        if (!error) {
            NSDictionary *responseDictionary = [NSJSONSerialization JSONObjectWithData:data options:kNilOptions error:&error];
            NSMutableArray *tempArray = [NSMutableArray array];
            for (NSDictionary *data in [responseDictionary valueForKeyPath:@"response.venues"]) {
                TNVenueObject *venueObject = [[TNVenueObject alloc] initVenueWithName:data[@"name"]
                                                                             location:data[@"location"]
                                                                              contact:data[@"contact"]
                                                                           identifier:data[@"id"]];
                [tempArray addObject:venueObject];

            }

            NSSortDescriptor *distanceSortDiscriptor = [NSSortDescriptor sortDescriptorWithKey:@"distance"
                                                                                     ascending:YES
                                                                                      selector:@selector(localizedStandardCompare:)];

            [tempArray sortUsingDescriptors:@[distanceSortDiscriptor]];
            _venueObject = [NSArray arrayWithArray:tempArray];
            [[NSNotificationCenter defaultCenter] postNotificationName:@"didFinishStoringVenueObject" object:nil];

        } else {
            NSLog(@"%@", error.localizedDescription);
        }
    });
}

- (NSDictionary *)mainCategoryKeys
{
    return  @{@"Arts & Entertainment" : @"4d4b7104d754a06370d81259",
              @"Colleges & Universities" : @"4d4b7105d754a06372d81259",
              @"Events" : @"4d4b7105d754a06373d81259",
              @"Food" : @"4d4b7105d754a06374d81259",
              @"Nightlife Spots" : @"4d4b7105d754a06376d81259",
              @"Outdoors & Recreation" : @"4d4b7105d754a06377d81259",
              @"Professional & Other Places" : @"4d4b7105d754a06375d81259",
              @"Residences" : @"4e67e38e036454776db1fb3a",
              @"Shops & Services" : @"4d4b7105d754a06378d81259",
              @"Travel & Transport" : @"4d4b7105d754a06379d81259"};
}

- (NSArray *)mainCategories
{
    return @[@"Arts & Entertainment",
             @"Colleges & Universities",
             @"Events",
             @"Food",
             @"Nightlife Spots",
             @"Outdoors & Recreation",
             @"Professional & Other Places",
             @"Residences",
             @"Shops & Services",
             @"Travel & Transport"];
}

- (void)performPhotoDetailsRequest:(NSString *)identifier
{
    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    dispatch_async(queue, ^{
        NSError *error = nil;
        NSURLResponse *response = nil;

        NSData *data = [NSURLConnection sendSynchronousRequest:[self buildRequestForVenuePhotoDetails:identifier] returningResponse:&response error:&error];

        if (!error) {
            NSDictionary *responseDictionary = [NSJSONSerialization JSONObjectWithData:data options:kNilOptions error:&error];
            NSMutableArray *tempArray = [NSMutableArray array];

            if ([[[responseDictionary valueForKeyPath:@"response.venue.photos.groups"] valueForKey:@"items"] count] > 0) {
                for (NSDictionary *data in [[responseDictionary valueForKeyPath:@"response.venue.photos.groups"] valueForKey:@"items"][0]) {
                    TNVenueImageData *imageData = [[TNVenueImageData alloc] initWithPrefix:data[@"prefix"]
                                                                                     width:data[@"width"]
                                                                                    height:data[@"height"]
                                                                                    suffix:data[@"suffix"]
                                                                                  userInfo:data[@"user"]];
                    [tempArray addObject:imageData];
                }
                _imageData = [NSArray arrayWithArray:tempArray];
                [[NSNotificationCenter defaultCenter] postNotificationName:@"didFinishStoringImageData" object:nil];
            } else {
                NSLog(@"No Images...");
            }

        } else {
            NSLog(@"%@", error.localizedDescription);
        }
    });
}

- (void)downloadImage:(TNVenueImageData *)imageData completionBlock:(TNVenueServiceCompletionBlock)completion
{
    NSError *error = nil;
    NSURL *urlString = [NSURL URLWithString:[self constructImageURL:imageData]];
    NSData *data = [NSData dataWithContentsOfURL:urlString options:NSDataReadingUncached error:&error];
    __block UIImage *image;

    dispatch_queue_t queue = dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0);
    dispatch_async(queue, ^{
        image = [UIImage imageWithData:data];
        completion(image, error);
    });
}

- (NSString *)getTodaysDate
{
    NSDate *date = [NSDate date];
    NSDateFormatter *formatter = [[NSDateFormatter alloc] init];
    [formatter setDateFormat:@"yyyMMdd"];

    return [formatter stringFromDate:date];

}

- (NSString *)constructImageURL:(TNVenueImageData *)imageData
{
    return [NSString stringWithFormat:@"%@%@x%@%@", imageData.photoPrefix,
            imageData.photoWidth, imageData.photoHeight, imageData.photoSuffix];
}

- (NSURLRequest *)buildRequestForVenueCategory
{
    return [self URLRequestWithFormat:@"https://api.foursquare.com/v2/venues/categories?client_id=%@&client_secret=%@&v=%@",
            _clientID, _clientSecret, [self getTodaysDate]];
}

- (NSURLRequest *)buildRequestForVenueLocation:(CLLocation *)location identifier:(NSString *)identifier
{
    return [self URLRequestWithFormat:@"https://api.foursquare.com/v2/venues/search?ll=%f,%f&radius=%@&categoryId=%@&client_id=%@&client_secret=%@&v=%@", location.coordinate.latitude, location.coordinate.longitude, _radius, identifier, _clientID, _clientSecret, [self getTodaysDate]];
}

- (NSURLRequest *)buildRequestForVenuePhotoDetails:(NSString *)identifier
{
    return [self URLRequestWithFormat:@"https://api.foursquare.com/v2/venues/%@?client_id=%@&client_secret=%@&v=%@",
            identifier, _clientID, _clientSecret, [self getTodaysDate]];
}


@end

Any further help as to what can be improved once more, would be very helpful! :)

Answer 1

First, I'll focus on some of the easy, obvious things I see. I know the main point of this code is some of the networking stuff, but there are some other things I want to comment on first.

---

First and foremost, this line:

@class TNVenueImageData;

It's fine to declare the class in the .h and wait until the .m to import the file... but you must have clipped off some lines, because I don't see the actual import anywhere.

But at the end of the day, it doesn't actually hurt to import the file in the .h. After all, anyone who imports this file is likely going to also import the other file. Double importing doesn't hurt (it's only imported once, no matter how many times you try). And now if I'm using this class, I could just import venueService and not have to import the image data file, because the venue service file is importing it for me.

---

Next, what's this stuff exactly?

venueService.clientID = @"MY_CLIENT_ID";
venueService.clientSecret = @"MY_CLIENT_SECRET";
venueService.radius = @"2000"; //start-radius

Surely "MY_CLIENT_..." can't be the actual values here. But the method is simply venueService and takes no arguments, so these are definitely constant values. They're also public properties.

So, first of all, we can start by defining these as constants within the .m file.

Outside of the @interface or @implementation, up near the imports, define some constants:

static NSString * const kVenueServiceClientID = @"MY_CLIENT_ID";
static NSString * const kVenueServiceClientSecret = @"MY_CLIENT_SECRET";
static NSString * const kVenueServiceRadius = @"2000";

Now if you need to use these values within the class, simple call the constant variable.

If you want to use the variables outside the class, you can create readonly properties for each.

In the .h, set up your properties as such:

@property (nonatomic,readonly) NSString *clientID;
@property (nonatomic,readonly) NSString *clientSecret;
@property (nonatomic,readonly) NSString *radius;

Now in the .m, overwrite the default accessor for each:

- (NSString *)clientID {
    return kVenueServiceClientID;
}

And repeat for the other two.

---

Now, as for these methods: mainCategories and mainCategoryKeys, I have a handful of problems.

The first problem is that the method names are confusing from an Objective-C standpoint. mainCategoryKeys returns a dictionary of values... while mainCategories returns an array which confusingly enough happens to be the same values you'd get if you called allKeys on the dictionary returned by mainCategoryKeys.

I assume the strange values in the value part of the dictionary are keys used with the API.

So, first of all, let's see what we can do to fix the method names here.

The method that returns a dictionary returns a dictionary, so how about: mainCategoryKeyDictionary, which makes it more clear that the values are actually keys to something else.

As for the other method, some name options include mainCategoryNames, or mainCategoryDescriptions, but honestly, the fact that an identical array can be produced simply by calling allKeys on the dictionary makes me think this method is kind of extraneous.

Now that we've got them renamed, I there are still other problems. First, constants, again. The same logic as above applies.

But the bigger issue I see here is that this should probably be a class level method rather than an instance method. In another language like Java, this might not even be a method, but just a public static variable. Objective-C doesn't offer class level variables, but there's a commonly used pattern to achieve the same effect in Objective-C. It looks like this:

+ (NSDictionary *)mainCategoryKeyDictionary {
    @synchronized(self) {
        static NSDictionary *keyDictionary;
        if (!keyDictionary) {
            keyDictionary = @{ /*stuff*/ };
        }
        return keyDictionary;
    }
}

Where /*stuff*/ is replace with the actual keys and values of course.

This is more efficient in terms of speed and memory usage.

---

As for the methods that do the actual networking, NSNotificationCenter is the 3rd best option out of 3 ways I know for doing what you're trying to do with it.

If you are going to use NSNotificationCenter, there's two important things you need to do with the notification name. First, use the reverse domain format for the actual name.

com.example.yourNotificationsActualName

The same naming scheme, by the way, needs to be applied to any threads you create. You don't want to accidentally register for some other notification that coincidentally has the same name you picked... and reverse domain naming is a good way to avoid that.

Second of all, these notification names absolutely must be defined constants available to anyone that has imported the .h for this file. How else will they know what notifications to register for when using your code?

But ultimately, these are two reasons why the other options are better.

The other options I know of are completion blocks and the delegate-protocol pattern.

First of all, based on your previous question, I know you have some experience with completion blocks. I'm personally not a huge fan of completion blocks, but they're there, and despite my personal bias, I consider them equally as good as protocol-delegate pattern.

The protocol-delegate pattern is something you're already familiar with in Objective-C even if you don't realize it.

A protocol-delegate setup consists of three parts.

• The protocol. There are lots of already existing protocols in Cocoa. One of the most common ones everyone gets familiar with is UITableViewDelegate and UITableViewData source. A protocol is simply a listing of required and/or optional method and/or property declarations. Objects can choose to conform to a protocol, and those that do are guaranteed to respond to any of the required methods and can potentially respond to the optional ones.

Here's an example of what a protocol definition looks like:

@protocol FooBarDelegate <NSObject>

@required - (void)fooBaringDidCompleteWithData:(NSData *)fooBarData;
@optional - (void)fooBaringDidFailWithError:(NSError *)error;

@end

So this is a protocol definition. The protocol name is MyExampleProtocol, and this protocol conforms to NSObject protocol. This simply means that this protocol includes EVERYTHING in the NSObject protocol as well as the listed stuff.

The protocol also includes two methods, one required, one optional.

• The delegator. The delegator is simply any class that has a reference to an object conforming to the protocol. The delegator has to know about the protocol. In most cases (personal experience), the delegator's file is the one that defines the protocol, though it's perfectly acceptable for it to be defined in another file and imported into the delegator's file.

Typically, the delegator will have a property called delegate, and it's typically (although not always, depending on what the delegate it totally needed for) defined simply as id (conforming to the protocol). Although you can be more specific as to what type the object is (UIViewController conforming to a specific protocol isn't completely uncommon). Example:

@property (nonatomic,**weak**) id<FooBarDelegate> delegate;

The weak is VERY important here. Using strong typically creates a retain cycle because typically the delegate has strong reference to the delegator. And weak is better than assign because weak will auto-zero preventing us from calling methods to a deallocated object.

• The delegate. In the @interface part of the delegate (either .h or .m, we have to define our delegate as conforming to the protocol.

Example:

@interface MyFooBarDelegateExampleClass <FooBarDelegate>

Now, Xcode will warn us that we're missing any method that FooBarDelegate defines as required, so we implement the required methods.

So, we've got an idea of the three parts. Let's go back to the delegator.

When some operation is complete and we're ready to let the delegate know, here's how we do it.

[self.delegate fooBaringDidCompleteWithData:myFooBarData];

We've called a method in that class and sent it the data we built in this class. It's now free to do whatever it wants with the data.

Now that was a required method. If it's an optional method, we need to do some checking first:

if ([self.delegate respondsToSelector:@selector(fooBaringDidFailWithError:)]) {
    [self.delegate fooBaringDidFailWithError:error];
}

Now, if the delegate has implemented this method, we've sent it the error information and it can do with it what it wants. If it didn't implement the method, we just move on.

---

Getting comfortable with the delegate-protocol pattern is very important in Objective-C and will help you think about a lot of problems in a much different way that can make lots of things much easier for you.