Classes representing backend service and persisted data in iOS app

Question

I just created 2 model class for my iOS app:

1. HALUserDefaults represents the persisted data in my app. It has the ability to both retrieve persisted data and store new data.

2. HALParseConnection represents the connection to my Backend service Parse.com

These 2 models were made to work with each other.

HALUserDefaults.h:

#import <Foundation/Foundation.h>

@interface HALUserDefaults : NSObject

#pragma mark - Instance Methods
- (NSArray *)retrieveHalfImageMessages;
- (NSArray *)retrieveFullImageMessages;
- (void)storeHalfImageMessages:(id)halfImageMessages;
- (void)storeFullImageMessages:(id)fullImageMessages;

@end

HALUserDefaults.m:

#import "HALUserDefaults.h"

@interface HALUserDefaults ()

@end

@implementation HALUserDefaults


- (NSArray *)retrieveHalfImageMessages
{
    NSUserDefaults *standardDefaults = [NSUserDefaults standardUserDefaults];
    NSData *data = [standardDefaults objectForKey:@"halfImageMessages"];
    NSArray *retrievedArray = [NSKeyedUnarchiver unarchiveObjectWithData:data];

    return retrievedArray;
}

- (NSArray *)retrieveFullImageMessages
{
    NSUserDefaults *standardDefaults = [NSUserDefaults standardUserDefaults];
    NSData *data = [standardDefaults objectForKey:@"fullImageMessages"];
    NSArray *retreivedArray = [NSKeyedUnarchiver unarchiveObjectWithData:data];

    return retreivedArray;
}

- (void)storeHalfImageMessages:(id)halfImageMessages
{
    NSUserDefaults *standardDefaults = [NSUserDefaults standardUserDefaults];
    [standardDefaults setObject:[NSKeyedArchiver archivedDataWithRootObject:halfImageMessages] forKey:@"halfImageMessages"];
    [standardDefaults synchronize];
}

- (void)storeFullImageMessages:(id)fullImageMessages
{
    NSUserDefaults *standardDefaults = [NSUserDefaults standardUserDefaults];
    [standardDefaults setObject:[NSKeyedArchiver archivedDataWithRootObject:fullImageMessages] forKey:@"fullImageMessages"];
    [standardDefaults synchronize];
}

@end

HALParseConnection.h:

#import <Foundation/Foundation.h>
#import <Parse/Parse.h>

@interface HALParseConnection : NSObject

#pragma mark - Instance Methods
- (void)performQuery;
- (void)performQuery2and3;

@end

HALParseConnection.m:

#import "HALParseConnection.h"
#import "HALUserDefaults.h"

@interface HALParseConnection ()

@end

@implementation HALParseConnection


- (void)performQuery
{
    // Setup and execute the query

    PFQuery *query = [PFQuery queryWithClassName:@"Messages"];
    [query whereKey:@"recipientIds" equalTo:[[PFUser currentUser]objectId]];
    [query whereKey:@"halfOrFull" equalTo:@"half"];
    [query whereKey:@"didRespond" notEqualTo:[[PFUser currentUser]objectId]];
    [query orderByDescending:@"createdAt"];

    [query findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
        if (error) {
            NSLog(@"There was an error: %@", error);
        } else {

            // Store the returned objects and post notification
            HALUserDefaults *userDefaults = [[HALUserDefaults alloc]init];
            [userDefaults storeHalfImageMessages:objects];
            [[NSNotificationCenter defaultCenter] postNotificationName:@"queryHasFinished"
                                                                object:self
                                                              userInfo:nil];
        }

    }];
}

- (void)performQuery2and3
{
    // Setup and execute the query
    PFQuery *query2 = [PFQuery queryWithClassName:@"Messages"];
    [query2 whereKey:@"senderId" equalTo:[[PFUser currentUser]objectId]];
    [query2 whereKey:@"halfOrFull" equalTo:@"full"];

    PFQuery *query3 = [PFQuery queryWithClassName:@"Messages"];
    [query3 whereKey:@"recipientIds" equalTo:[[PFUser currentUser]objectId]];
    [query3 whereKey:@"halfOrFull" equalTo:@"full"];

    PFQuery *query2and3 = [PFQuery orQueryWithSubqueries:[NSArray arrayWithObjects:query2,query3,nil]];
    [query2and3 orderByDescending:@"createdAt"];

    [query2and3 findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
        if (error) {
            NSLog(@"There was an error: %@", error);
        } else {

            // Store the returned objects and post notification
            HALUserDefaults *userDefaults = [[HALUserDefaults alloc]init];
            [userDefaults storeFullImageMessages:objects];
            [[NSNotificationCenter defaultCenter] postNotificationName:@"query2and3HasFinished"
                                                                object:self
                                                              userInfo:nil];
        }
    }];
}

@end

Here is a small sample of my View Controller file, to show I am using my 2 new classes:

- (void)viewWillAppear:(BOOL)animated
{
    [super viewWillAppear:animated];

    [self retreiveStoredMessages];

    [self parseQueries];
}

#pragma mark - Stored Data Methods
- (void)retreiveStoredMessages
{
    self.halfImageMessages = [[NSArray alloc]init];
    self.fullImageMessages = [[NSArray alloc]init];
    HALUserDefaults *userDefaults = [[HALUserDefaults alloc]init];

    // Retreive any stored messages
    self.halfImageMessages = [userDefaults retrieveHalfImageMessages];
    self.fullImageMessages = [userDefaults retrieveFullImageMessages];

    // Reload table view
    [self.tableView reloadData];
}


#pragma mark - Parse Query Methods
- (void)parseQueries
{
    // Execute Parse queries
    HALParseConnection *parseConnection = [[HALParseConnection alloc]init];
    [parseConnection performQuery];
    [parseConnection performQuery2and3];

    // Register for notifications that are posted in HALParseConnection class
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(parseQueryFinished)
                                                 name:@"queryHasFinished"
                                               object:nil];
    [[NSNotificationCenter defaultCenter] addObserver:self
                                             selector:@selector(parseQuery2and3Finished)
                                                 name:@"query2and3HasFinished"
                                               object:nil];
}

- (void)parseQueryFinished
{
    // Retreive newly updated halfImageMessages and remove observer
    HALUserDefaults *userDefaults = [[HALUserDefaults alloc]init];
    self.halfImageMessages = [userDefaults retrieveHalfImageMessages];
    [[NSNotificationCenter defaultCenter] removeObserver:self
                                                    name:@"queryHasFinished"
                                                  object:nil];
    [self.tableView reloadData];
}

- (void)parseQuery2and3Finished
{
    // Retreive newly updated fullImageMessages and remove observer
    HALUserDefaults *userDefaults = [[HALUserDefaults alloc]init];
    self.fullImageMessages = [userDefaults retrieveFullImageMessages];
    [[NSNotificationCenter defaultCenter] removeObserver:self
                                                    name:@"query2and3HasFinished"
                                                  object:nil];
    [self.tableView reloadData];
}

Here are some of the key things that I am looking for feedback on:

1. MVC - The main point of these 2 new classes is to move my app towards the MVC design pattern. I want to make sure that this code represents and implements MVC well.

2. Naming conventions - I want to make sure that the names I am choosing for my methods, classes, and properties are following best practices.

3. Notifications - I want to make sure that I am properly using NSNotificationCenter. You will notice that in the HALParseConnection class that I am posting notifications inside my blocks when there are no errors with the findObjectsInBackgroundWithBlock method calls.

   I post these notifications because I need to know when the asynchronous method has been completed.

   You will also notice that I register my View Controller for these notifications, and execute various methods in response to the notifications being posted.

In regards to my use of NSUserDefaults to persist data, I already understand that I should be using Core Data instead.

Answer 1

HALUserDefaults

First of all, the calls to synchronize can probably be eliminated. It's quite rare that you actually have to make this call in order for the data to properly persist, and making the call every time you store something in defaults completely eliminates any advantage you'd gain by using NSUserDefaults.

NSUserDefaults persists the stored data in memory until the device can find an efficient time to write the data to permanent memory on a background thread. Calling synchronize forces the app to write to permanent storage right then and there.

---

But there's a bigger problem with HALUserDefaults. The 4 methods that exist within the class are written as instance methods, but they're stateless methods. Their function doesn't depend on any thing about the state of the instance of the HALUserDefaults. And even if it did, there's no state to set.

Furthermore, the only reason to instantiate a HALUserDefaults object is to call these methods.

There are two solutions here. We can swap these over to class methods (change the - to a +) so we still maintain the "namespace" of HALUserDefaults, or we can change them to C-Style functions.

---

This is something first noticed in HALUserDefaults but applies to all instances where you're using a literal string as a key or notification name (or a lot of other things). If there's even a small possibility that you'll use a string literal more than once, it should be declared as a NSString * const and used that way. This is especially true for literal strings used across multiple files (such as the notification names).

---

HALParseQuery

Your query methods need better names. What kind of query are they performing?

But for me, the bigger problem with this class is how we're handling the results of the asynchronous request.

For starters, we've got the same problem as HALUserDefaults where we have instance methods that don't depend on the state of the object... and we have a class that doesn't have anything specific to the instance at all. So again, we're better off setting these as class methods of C-style functions. But honestly, for my money, I'd prefer seeing this left as an instance method and giving the class a protocol, and using the protocol-delegate pattern rather than NSNotificationCenter.

The protocol, declared in the .h might look something like this:

@protocol HALParseConnectionDelegate

@required - (void)halParseConnection:(HALParseConnection *)halParseConnection
              didCompleteWithResults:(NSArray *)results
                               error:(NSError *)error;

@end

Now the async block here:

[query2and3 findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    if (error) {
        NSLog(@"There was an error: %@", error);
    } else {

        // Store the returned objects and post notification
        HALUserDefaults *userDefaults = [[HALUserDefaults alloc]init];
        [userDefaults storeFullImageMessages:objects];
        [[NSNotificationCenter defaultCenter] postNotificationName:@"query2and3HasFinished"
                                                            object:self
                                                          userInfo:nil];
    }
}];

is instead replaced with simply this:

[query2and3 findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    [self.delegate halParseConnection:self
               didCompleteWithResults:objects
                                error:error];
}];  

This lets the delegate do what it wants with the results and choose how to respond to an error.

Alternatively, you could switch these to class methods (as I recommended for HALUserDefaults) and keep them mostly the same, but with a couple of changes.

First of all, strip out the saving them to user defaults here. We shouldn't bother doing that. It should be up to the object that requested the data to determine whether or not the results should be saved to user defaults.

Second, you need a new notification for when the request fails.

Third, pass the data with the notification!

So your async request will look more like this:

[query2and3 findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    NSNotificationCenter *notifCenter = [NSNotificationCenter defaultCenter];
    if (error) {
        [notifCenter postNotificationName:kQuery2And3Failed
                                   object:self
                                 userInfo:@{kQuery2And3Error : error}];
    } else {
        [notifCenter postNotificationName:kQuery2And3Complete
                                   object:self
                                 userInfo:@{kQuery2And3Results : objects}];
    }
}];

Where kQuery2And3Failed, kQuery2And3Error, kQuery2And3Complete, and kQuery2And3Results are all defined const strings.

---

Example usage with the recommended changes

If we go with the protocol-delegate style HALParseQuery, example usage would look something like this:

Performing the queries:

HALParseConnection *parseConnection1 = [[HALParseConnection alloc]init];
parseConnection1.delegate = self;
parseConnection1.tag = 100;
[parseConnection1 performQuery];

HALParseConnection *parseConnection2 = [[HALParseConnection alloc]init];
parseConnection2.delegate = self;
parseConnection2.tag = 101;
[parseConnection2 performQuery2and3];

Implementing the delegate method:

- (void)halParseConnection:(HALParseConnection *)halParseConnection
    didCompleteWithResults:(NSArray *)results
                     error:(NSError *)error {
    if (error) {
        NSLog(@"There was an error: %@", error);
    } else {
        if (halParseConnection.tag == 100) {
            // stuff
        } else {
            // stuff
        }
    }
}

Alternatively, if you fix up your notification method as I recommended, you'll have to change the methods that respond to the notifications to take an NSNotification object.

- (void)query2And3Complete:(NSNotification *)aNotification {
    NSDictionary *userInfo = aNotification.userInfo;
    self.fullImageMessages = userInfo[kQuery2And3Results];
    [HALUserDefaults storeFullImageMessages: self.fullImageMessages];
    [self.tableView reloadData];
}

And if you're going to stick with notifications, make sure you unregister for the notifications in dealloc! Otherwise the notification center may try sending messages to your deallocated object, and that's a crash every time.

- (void)dealloc {
    [[NSNotificationCenter defaultCenter] removeObserver:self];
}