Address book with singletons

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

@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
retu

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.

StackExchange Code Review Q#58708, answer score: 7