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

Model class for networking that uses notifications

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

Problem

I just finished a model class that represents an Instagram Networking client for my iOS app.

I am specifically looking for a review of my use of static constants. These are strings that represent notification names and notification userInfo dictionary keys.

Here is my networking class:

POPInstagramNetworkingClient.h:

#import "AFHTTPSessionManager.h"
@interface POPInstagramNetworkingClient : AFHTTPSessionManager

+ (id)sharedPOPInstagramNetworkingClient;

- (instancetype)initWithBaseURL:(NSURL *)url;
- (void)requestPopularMedia;
- (void)requestMediaWithTag:(NSString *)tag;

@end


POPInstagramNetworkingClient.m:

```
#import "POPInstagramNetworkingClient.h"
#import

@implementation POPInstagramNetworkingClient

#pragma mark - Singleton
+ (instancetype)sharedPOPInstagramNetworkingClient
{
static POPInstagramNetworkingClient *sharedPOPInstagramNetworkingClient = nil;
static dispatch_once_t onceToken;
dispatch_once(&onceToken, ^{

//Define base URL string
static NSString * const BaseURLString = @"https://api.instagram.com/v1/";

//Create our shared networking client for Instagram with base URL
sharedPOPInstagramNetworkingClient = [[POPInstagramNetworkingClient alloc]initWithBaseURL:[NSURL URLWithString:BaseURLString]];
});

return sharedPOPInstagramNetworkingClient;
}

#pragma mark - Initializer Methods
  • (instancetype)initWithBaseURL:(NSURL *)url


{
self = [super initWithBaseURL:url];

if (self) {
//Set the serializers
self.requestSerializer = [AFJSONRequestSerializer serializer];
self.responseSerializer = [AFJSONResponseSerializer serializer];
}

return self;
}

#pragma mark - Instance Methods
  • (void)requestPopularMedia


{
//Define notification names
static NSString * const kRequestForPopularMediaSuccessful = @"RequestForPopularMediaSuccessful";
static NSString * const kRequestForPopularMediaUnsuccessful = @"RequestForPopularMediaUnsuccessful";

//

Solution

//Define notification names
static NSString * const kRequestForPopularMediaSuccessful = @"RequestForPopularMediaSuccessful";
static NSString * const kRequestForPopularMediaUnsuccessful = @"RequestForPopularMediaUnsuccessful";


The scope of a notification name should never be limited to a single method or function.

The whole point of notification is that one object posts the notification, and as many objects who are registered can receive the notification.

Moreover, you've defined these identical constants in multiple methods.

So, first of all, if you use it in more than one method, it variable should at least be scoped to the class or file you're using it in. Redefining it every time you use it absolutely defeats the purpose of having a variable at all.

Second, this variable actually needs to be defined in the .h file. It's a notification name. If other objects are going to register for this notification, they need to know the name of the notification, and the best way to let them know the notification name is by giving them a variable in the .h file to use rather than having them look up the notification string.

//Define base URL string
static NSString * const BaseURLString = @"https://api.instagram.com/v1/";

//Create our shared networking client for Instagram with base URL
sharedPOPInstagramNetworkingClient = [[POPInstagramNetworkingClient alloc]initWithBaseURL:[NSURL URLWithString:BaseURLString]];


In this case, since we're only ever using this string once, it's fine to just use the literal string directly in the method call.

The point of declaring a variable like this would be if we reused it multiple times within the method--as it stands, we don't re-use it, so we don't need to create the variable.

Though... if we do create a variable, it should follow proper camelCase naming conventions.

Code Snippets

//Define notification names
static NSString * const kRequestForPopularMediaSuccessful = @"RequestForPopularMediaSuccessful";
static NSString * const kRequestForPopularMediaUnsuccessful = @"RequestForPopularMediaUnsuccessful";
//Define base URL string
static NSString * const BaseURLString = @"https://api.instagram.com/v1/";

//Create our shared networking client for Instagram with base URL
sharedPOPInstagramNetworkingClient = [[POPInstagramNetworkingClient alloc]initWithBaseURL:[NSURL URLWithString:BaseURLString]];

Context

StackExchange Code Review Q#61110, answer score: 4

Revisions (0)

No revisions yet.