Address book with singletons

Question

My main goal with this refactor was to change instance methods that should have been class methods in the first place into class methods. My secondary goal was to add singletons to these classes because they are Model classes and I feel that there should only be one instance at any given time.

HALAddressBook.h:

#import <AddressBook/AddressBook.h>
#import <AddressBookUI/AddressBookUI.h>
#import <AddressBook/ABAddressBook.h>
#import <AddressBook/ABPerson.h>

@interface HALAddressBook : NSObject

#pragma mark - Properties
@property NSArray *contacts;

#pragma mark - Instance Methods
- (BOOL)isAccessGranted;

#pragma mark - Singleton Method
+ (HALAddressBook *)sharedHALAddressBook;

@end

HALAddressBook.m:

#import "HALAddressBook.h"

@implementation HALAddressBook

#pragma mark - Singleton Method
+ (HALAddressBook *)sharedHALAddressBook
{
    static HALAddressBook *sharedHALAddressBook = nil;

    static dispatch_once_t onceToken;
    dispatch_once(&onceToken, ^{
        sharedHALAddressBook = [[self alloc]init];
    });

    return sharedHALAddressBook;
}

- (BOOL)isAccessGranted
{

    ABAddressBookRef m_addressbook =  ABAddressBookCreateWithOptions(NULL, NULL);

    __block BOOL accessGranted = NO;
    if (ABAddressBookRequestAccessWithCompletion != NULL) {
        dispatch_semaphore_t sema = dispatch_semaphore_create(0);
        dispatch_async(dispatch_get_global_queue(DISPATCH_QUEUE_PRIORITY_DEFAULT, 0), ^{
            @autoreleasepool {
                // Write your code here...
                // Fetch data from SQLite DB
            }
        });


        ABAddressBookRequestAccessWithCompletion(m_addressbook, ^(bool granted, CFErrorRef error)
        {
          accessGranted = granted;

          dispatch_semaphore_signal(sema);
                        });

        dispatch_semaphore_wait(sema, DISPATCH_TIME_FOREVER);
    }

    if (accessGranted) {
        // Access has been granted
       self.contacts = (__bridge NSArray *)(ABAddressBookCopyArrayOfAllPeople(m_addressbook));

        return YES;

    } else {
    // Access has not been granted
    return NO;
    }
}
@end

HALContact.h:

#import "HALAddressBook.h"

@interface HALContact : NSObject

#pragma mark - Properties
@property NSArray *phoneNumbers;
@property NSString *mainPhoneNumber;
@property NSString *firstName;
@property ABRecordRef contactRef;

#pragma mark - Instance Methods
- (BOOL)hasMultiplePhoneNumbers;

@end

HALContact.m:

#import "HALContact.h"

@interface HALContact ()

@end

@implementation HALContact

- (BOOL)hasMultiplePhoneNumbers
{
    if (self.phoneNumbers.count > 1) {
        return YES;
    } else {
        return NO;
    }
}
@end

HALUserDefaults.h:

@interface HALUserDefaults : NSObject

#pragma mark - Class Methods
+ (NSArray *)retrieveHalfImageMessages;
+ (NSArray *)retrieveFullImageMessages;
+ (void)storeHalfImageMessages:(id)halfImageMessages;
+ (void)storeFullImageMessages:(id)fullImageMessages;
+ (void)storeUsername:(NSString *)username;

@end

HALUserDefaults.m:

#import "HALUserDefaults.h"

@interface HALUserDefaults ()

@end

@implementation HALUserDefaults

#pragma  mark - Singleton Method
+ (HALUserDefaults *)sharedUserDefaults
{
    static HALUserDefaults *sharedUserDefaults = nil;

    dispatch_once_t onceToken;
    dispatch_once(&onceToken, ^{

        sharedUserDefaults = [[self alloc]init];
    });

    return sharedUserDefaults;
}

+ (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"];
}

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

+ (void)storeUsername:(NSString *)username
{
    NSUserDefaults *standardDefaults = [NSUserDefaults standardUserDefaults];

    [standardDefaults setObject:username forKey:@"username"];
}

@end

HALParseConnection.h:

#import <Parse/Parse.h>
@interface HALParseConnection : NSObject

#pragma mark - Class Methods
+ (void)performHalfImageQuery;
+ (void)performFullImageQuery;
+ (void)signupNewUserWithUsername:(NSString *)username password:(NSString *)password email:(NSString *)email;
+ (void)loginUserWithUsername:(NSString *)username password:(NSString *)password;
+ (void)performFriendsRelationForCurrentUserQuery;
+ (void)isUsernameAvailable:(NSString *)lowercaseUsername;

@end

HALParseConnection.m:

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

@interface HALParseConnection ()

@end

@implementation HALParseConnection


#pragma mark - Query Methods
+ (void)performHalfImageQuery
{
    // 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 storeHalfImageMessages:objects];
            [[NSNotificationCenter defaultCenter] postNotificationName:@"queryHasFinished"
                                                                object:self
                                                              userInfo:nil];
        }

    }];
}

+ (void)performFullImageQuery
{
    // 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 storeFullImageMessages:objects];
            [[NSNotificationCenter defaultCenter] postNotificationName:@"query2and3HasFinished"
                                                                object:self
                                                              userInfo:nil];
        }
    }];
}

+ (void)performFriendsRelationForCurrentUserQuery
{
    PFRelation *friendsRelation = [[PFUser currentUser]objectForKey:@"friendsRelation"];
    PFQuery *query = [friendsRelation query];
    [query orderByAscending:@"username"];
    [query findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {

        if(error) {

            [[NSNotificationCenter defaultCenter]postNotificationName:@"performFriendsRelationForCurrentUserQueryFailed" object:self userInfo:@{@"error" : error}];

        } else {

            [[NSNotificationCenter defaultCenter]postNotificationName:@"performFriendsRelationForCurrentUserQuerySucceeded" object:self userInfo:@{@"succeeded" : objects}];
        }

    }];

}

#pragma mark - Signup Methods
+ (void)signupNewUserWithUsername:(NSString *)username password:(NSString *)password email:(NSString *)email
{
    // Create new parse user
    PFUser *newUser = [PFUser user];
    newUser.username = username;
    newUser.password = password;
    newUser.email = email;


    // Signup new parse user
    [newUser signUpInBackgroundWithBlock:^(BOOL succeeded, NSError *error) {

    if (error) {

        NSLog(@"There was an error when trying to signup the new user: %@", error);

        // Post notification for unsuccessful signup
        [[NSNotificationCenter defaultCenter] postNotificationName:@"unsuccessfulUserSignup"
                                                            object:self
                                                          userInfo:nil];

        } else {

            // Persist user's username            
            [HALUserDefaults storeUsername:username];

            // Post notification for signup completion
            [[NSNotificationCenter defaultCenter] postNotificationName:@"successfulUserSignup"
                                                                object:self
                                                              userInfo:nil];
        }
    }];
}

+ (void)isUsernameAvailable:(NSString *)lowercaseUsername
{
    PFQuery *query = [PFQuery queryWithClassName:@"_User"];
    [query whereKey:@"lowercaseUsername" equalTo:lowercaseUsername];
    [query findObjectsInBackgroundWithBlock:^(NSArray *objects, NSError *error) {

        if (error) {
            NSLog(@"There was an error when querying Parse for the lowercaseUsername key");
        } else if (objects.count == 0) {
            [[NSNotificationCenter defaultCenter]postNotificationName:@"usernameIsAvailable" object:self];
        } else if (objects.count == 1) {
            [[NSNotificationCenter defaultCenter]postNotificationName:@"usernameIsNotAvailable" object:self];
        }
    }];
}

#pragma mark - Login Methods
+ (void)loginUserWithUsername:(NSString *)username password:(NSString *)password
{

    [PFUser logInWithUsernameInBackground:username password:password block:^(PFUser *user, NSError *error) {

        if (error) {

            [[NSNotificationCenter defaultCenter] postNotificationName:@"unsuccessfulUserLogin"
                                                                object:self
                                                              userInfo:nil];
        }

        if (user) {

            // Persist username
            [HALUserDefaults storeUsername:user.username];

            // Post notification for signup completion
            [[NSNotificationCenter defaultCenter] postNotificationName:@"successfulUserLogin"
                                                                object:self
                                                              userInfo:nil];
        }
    }];

}

@end

Some of the key things that I am looking for feedback on:

1. Do I have too many class methods? Do I have class methods that should/could be instance methods?

   One of the things I'm having trouble with is deciding when a method should be a class method or an instance method. I understand that usually a class method is for constructing an instance or a singleton, and that an instance method is for performing operations on a particular instance, but I am having trouble differentiating in my model classes for some reason.

2. Am I correct in thinking that Model Classes should usually employ the singleton pattern?

3. You will notice that I am using a singleton in HALUserDefaults and HALAddressBook.

   Is it correct that I am using a static pointer and setting it to nil at the beginning of the method? Because I don't really understand why that is necessary.

   Is it correct that I am returning HALUserDefaults and HALAddressBook in the singleton's method signature? Because I have noticed some developers like to use id instead.

4. Is it pointless to add a singleton method to a class that only has class methods?

   I am conflicted on this because even if a class only has class methods, you can still create an instance of the class. So in the case that a class has all class methods, is it pointless to create a singleton, or is it still smart to create a singleton?

5. Last but not least, anything in general that you see that could be improved upon.

Answer 1

Do I have too many class methods? Do I have class methods that should/could be instance methods?

There's not a specific number of class methods you should have. Don't try counting your class methods or your instance methods. Class methods do class level things and instance methods do instance level things.

One thing that's for certain, a class that has no instance methods really doesn't need to be a class at all, and it definitely doesn't need a singleton method.

For example, HALUserDefaults doesn't need to be a class and doesn't need a singleton. It's just a set of convenience function laid over the top of NSUserDefaults.

We can instead just write a set of convenience functions in a file called HALUserDefaults, and we can even give these functions all HALUserDefaults names.

HALUserDefaults.h

@import Foundation;

NSArray * HALUserDefaultsHalfImageMessages();
NSArray * HALUserDefaultsFullImageMessages();
void storeHalfImageMessages(id halfImageMessages);
void storeFullImageMessages(id fullImageMessages);
void storeUsername(NSString *username);

HALUserDefaults.m

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"];
}

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

void storeUsername(NSString * username) {
    NSUserDefaults *standardDefaults = [NSUserDefaults standardUserDefaults];

    [standardDefaults setObject:username forKey:@"username"];
}

Now we just #import HALUserDefaults.h and use these as regular C-style functions. No need to take up memory space instantiating an object for any reason whatsoever.

The reason NSUserDefaults has the singleton method sharedUserDefaults is because it keeps your freshly written and recently read values in memory. It doesn't write them to disk until it synchronizes (which can be done by calling synchronize--but this is almost always unnecessary, it will write it in it's own time). The singleton exists to be sure that across you're app, you're accessing the same values without having to read/write from/to permanent storage every time you want to change or retrieve a value.

This same logic can most likely be applied to HALParseConnection as well.

---

+ (void)isUsernameAvailable:(NSString *)lowercaseUsername;

A method whose name starts with "is" should without exception return a BOOL value. This method can't because it's checking the availability asynchronously. As such, the method should likely be named something more like checkUsernameAvailability:.

And the argument name should just be username, not lowercaseUsername. If you need the user name to be lowercase in this method, that should be the first line of the method--you shouldn't expect the caller to lowercase it for you.

---

As a general comment, for your specific usage here, I really think you need to move away from using NSNotificationCenter here and implement this either with the protocol-delegate pattern or with completion/error blocks.

Answer 2

After thinking about this problem some more... creating the C-style functions as I recommended for my previous answer seems a good-ish solution to the HALParseConnection class... but HALUserDefaults is probably better served as a class category on NSUserDefaults.

NSUserDefaults+HALUserDefaults.h

@interface NSUserDefaults (HALUserDefaults)

- (NSArray *)retrieveHalfImageMessages;
- (NSArray *)retrieveFullImageMessages;
- (void)storeHalfImageMessages:(id)halfImageMessages;
- (void)storeFullImageMessages:(id)fullImageMessages;
- (void)storeUsername:(NSString *)username;

@end

NSUserDefaults+HALUserDefaults.m

@implementation NSUserDefaults (HALUserDefaults)

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

    return retrievedArray;
}

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

    return retreivedArray;
}

- (void)storeHalfImageMessages:(id)halfImageMessages {
    [self setObject:[NSKeyedArchiver archivedDataWithRootObject:halfImageMessages] forKey:@"halfImageMessages"];
}

- (void)storeFullImageMessages:(id)fullImageMessages {
    [self setObject:[NSKeyedArchiver archivedDataWithRootObject:fullImageMessages] forKey:@"fullImageMessages"];
}

- (void)storeUsername:(NSString *)username {
    [self setObject:username forKey:@"username"];
}

@end

And now you've just added 5 methods to NSUserDefaults.

NSUserDefaults *defaults = [NSUserDefaults standardUserDefaults];

[defaults storeUsername:@"nhgrif"];

NSArray *myArray = [defaults retrieveHalfImageMessages];