patternMinor
Classes representing backend service and persisted data in iOS app
Viewed 0 times
backendiospersistedappserviceclassesanddatarepresenting
Problem
I just created 2 model class for my iOS app:
-
-
These 2 models were made to work with each other.
HALUserDefaults.h:
HALUserDefaults.m:
HALParseConnection.h:
```
#import
#import
@in
-
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.comThese 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;
@endHALUserDefaults.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];
}
@endHALParseConnection.h:
```
#import
#import
@in
Solution
HALUserDefaultsFirst 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).HALParseQueryYour 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;
@endNow 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 somethCode 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.