HiveBrain v1.2.0
Get Started
← Back to all entries
patternMinor

Classes representing backend service and persisted data in iOS app

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
backendiospersistedappserviceclassesanddatarepresenting

Problem

I just created 2 model class for my iOS app:

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

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

These 2 models were made to work with each other.

HALUserDefaults.h:

#import 

@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
#import

@in

Solution

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 someth

Code Snippets

@protocol HALParseConnectionDelegate

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

@end
[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];
    }
}];
[query2and3 findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {
    [self.delegate halParseConnection:self
               didCompleteWithResults:objects
                                error:error];
}];
[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}];
    }
}];
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];

Context

StackExchange Code Review Q#57028, answer score: 4

Revisions (0)

No revisions yet.