remote data fetching inside model object in objective c using AFNetworking

Question

In all of my iOS application I use this approach to respect MVC, I want to be sure that my implementation is correct and respects the best practices and the MVC design pattern :

Singleton of AFNetworking acting as API for network calls:

MyAPI.h :

#import "AFHTTPSessionManager.h"
#import "AFNetworking.h"

@interface MyAPI : AFHTTPSessionManager

+(MyAPI *)sharedInstance;

@end

MyAPI.m :

#pragma mark - Singleton

+(MyAPI*)sharedInstance
{
 static MyAPI *sharedInstance = nil;
 static dispatch_once_t onceToken;
 dispatch_once(&onceToken, ^{
    sharedInstance = [[MyAPI alloc] initWithBaseURL:[NSURL URLWithString:kROOT_URL]];
});
  return sharedInstance;
}

Model User that uses the singleton to fecth the data of user (is that good as implementation ?):

User.h

 @interface User : NSObject

 @property (strong,nonatomic) NSString *userId;
 @property (strong,nonatomic) NSString *email;
 @property (strong,nonatomic) NSString *password;


-(id) initWithDictionary: (NSDictionary *) dictionay;

 +(BOOL) isConnected;
 +(void) disconnect;
 +(NSString *) idOfConnectedUser;
 +(User *) connectedUser;

 +(void) loginWith : (NSString *) email andPassword :(NSString *) password complete:(void(^)(id result, NSError *error))block;
 +(void) searchUsersFrom : (NSString *) countryCode withName :(NSString *) name andLevel:(NSString *) levelCode complete: (void(^)(id result, NSError *error)) block;
 +(void) signup:(void(^)(id result, NSError *error)) block;
 +(void) getUserFriends:(void(^)(id result, NSError *error)) block;

@end

User.m

  [......]

 +(void) loginWith : (NSString *) email andPassword :(NSString *) password complete: (void(^)(id result, NSError *error)) block
 {

 __block NSString * result ;

NSDictionary *params = @{@"email": email, @"password": password};

[[MyAPI sharedInstance] POST:@"auth/" parameters:params success:^(NSURLSessionDataTask *task, id responseObject)
{

    if([responseObject objectForKey:@"id"])
    { 
        [[NSUserDefaults standardUserDefaults] setObject:(NSDictionary*) responseObject forKey:USER_KEY];
        [[NSUserDefaults standardUserDefaults] synchronize];
        result = [responseObject objectForKey:@"id"];
    }
    else
    {
        result = nil ;
    }


    if (block) block(result, nil);

} failure:^(NSURLSessionDataTask *task, NSError *error)
{
     if (block) block(nil, error);
}];

}
[.....]

LoginController.m :

-(void)loginButtonAction:(UIButton *)sender
{

    [......]

    [ User loginWith:text andPassword:text complete:^(id result, NSError *error)
     {
         if (result)
         {
             [APPDELEGATE start];
         }
         else
         {
          // ERROR
         }
       }];

   }

So does my implementation respects the MCV and follows the best practices and how can I improve it if not ?

Answer 1

Singletons: You might want to avoid using singletons, it'll help you to improve your API design and make code more testable. Also, in case of User, imagine you will want to support changing user (logout/guest user/etc). With current approach, you will be limited to sending a NSNotification because everyone who uses connectedUser can not know that underlying reference has changed.

ActiveRecord: What you did with your model User that is capable of performing networking is somewhat similar to active record approach which might not scale so well when you model becomes more complicated and the number of actions it can perform increases. Consider separating those into pure model and services that actually perform networking (or whatever else you will need in the future).

Model Serialisation: Consider encapsulating model & network response serialisation logic into a separate class (e.g. LoginResponse that among other things points to a User) frameworks like Mantle make it much easier.

MVC: from my experience in iOS MVC might not be the most optimal approach for anything but simple apps. With MVC tendency is to put put all the logic into your ViewController making it very big and hard to maintain. Consider other patterns such as MVVM

All in all I understand that it is hard to learn all the new technologies at once, but you can definitely start by making sure each class performs one thing and one thing only: Model does not do networking or persisting to the disk, API client doesn't deserialise each response or saves data to NSUserDefaults, view controller doesn't do anything except for listening to user events (button taps etc). This alone would make your code much easier to reason about and to follow if a new developer would be introduced to your codebase.

Hope it helps!