4
\$\begingroup\$

I've found myself wanting an easy way to implement chat into various apps, so I developed a set of classes for the firebase.com backend that make it easy for me to quickly set up the nuts and bolts of a chat system. To implement this, I have three classes:

FSPresenceManager

Current Connection / Other User's Connection

FSChannelManager

Send / Receive Alerts From users

FSChatManager

Create Chat / Send & Receive Messages / Light "Queries" (hoping to improve)

These classes act as sort of a notification center where you can register observers for callbacks and the class distributes the information when it comes in from firebase. For this reason, I used singletons in all cases. I decided to wrap all of these classes into a greater class, which is also a Singleton that I called FireSuite.

This allows me to set things like URL, and CurrentUserId for all managers at once in a few lines.

FireSuite.h

#import "FSChatManager.h"
#import "FSPresenceManager.h"
#import "FSChannelManager.h"

@interface FireSuite : NSObject

+ (FireSuite *) suiteManager; // Singleton

@property (strong, nonatomic) NSString * firebaseURL;
@property (strong, nonatomic) NSString * currentUserId;

@property (strong, nonatomic) FSChatManager * chatManager;
@property (strong, nonatomic) FSPresenceManager * presenceManager;
@property (strong, nonatomic) FSChannelManager * channelManager;

@end

FireSuite.m

#import "FireSuite.h"

@implementation FireSuite

@synthesize firebaseURL, currentUserId, presenceManager, chatManager, channelManager;

+ (FireSuite *) suiteManager {
    static dispatch_once_t pred;
    static FireSuite *shared = nil;

    dispatch_once(&pred, ^{
        shared = [[FireSuite alloc] init];
    });
    return shared;
}

#pragma mark SETTERS

- (void) setFirebaseURL:(NSString *)firebaseURLsetter {

    if (![firebaseURLsetter hasSuffix:@"/"]) {
        firebaseURLsetter = [NSString stringWithFormat:@"%@/", firebaseURLsetter];
    }

    // Set Our Tools
    [FSChatManager singleton].urlRefString = firebaseURLsetter;
    [FSPresenceManager singleton].urlRefString = firebaseURLsetter;
    [FSChannelManager singleton].urlRefString = firebaseURLsetter;

    firebaseURL = firebaseURLsetter;
}

- (void) setCurrentUserId:(NSString *)currentUserIdToSet {

    // Set Our Tools
    [FSChatManager singleton].currentUserId = currentUserIdToSet;
    [FSPresenceManager singleton].currentUserId = currentUserIdToSet;
    [FSChannelManager singleton].currentUserId = currentUserIdToSet;

    currentUserId = currentUserIdToSet;
}

#pragma mark GETTERS

- (FSPresenceManager *) presenceManager {
    FSPresenceManager * manager = [FSPresenceManager singleton];
    return manager;
}

- (FSChatManager *) chatManager {
    FSChatManager * manager = [FSChatManager singleton];
    return manager;
}

- (FSChannelManager *) channelManager {
    FSChannelManager * manager = [FSChannelManager singleton];
    return manager;
}

@end

Everything is still fairly "Stream Of Thought Programming" so there's a bit of inconsistency and things I'd prefer to implement differently, but before I proceed I wanted to get a second opinion on this.

The main reason is that it allows me to easily update firebaseURL / currentUserId one time without having to worry about it being up to date everywhere.

Aside from general preferences, is this going to create unforeseen problems?

Second opinions and alternative strategies are also welcome! Also, if there's problems with something about my post, please elaborate when downvoting. This is my first codeReview post and I might just not understand something!

Usage

Step 1: Create suiteManager and assign URL / CurrentUser

FireSuite * fireSuite = [FireSuite suiteManager];
fireSuite.firebaseURL = @"https://someFirebase.firebaseIO.com/";
fireSuite.currentUserId = @"currentUserId";

Step 2: Monitor Our Connection / Other User's Connection

FSPresenceManager * presenceManager = fireSuite.presenceManager;

// Start Monitor
[presenceManager startPresenceManager];

// Monitor Current User's Connection
[presenceManager registerConnectionStatusObserver:self withSelector:@selector(isConnected:)];

// Monitor Other Users (for instance, a chat opponent)
[presenceManager registerUserStatusObserver:self
                               withSelector:@selector(userStatusDidUpdateWithId:andStatus:)
                                  forUserId:@"userId1"];
[presenceManager registerUserStatusObserver:self
                               withSelector:@selector(userStatusDidUpdateWithId:andStatus:)
                                  forUserId:@"userId2"];

// Receive Presence Manager Notifications
- (void) isConnected:(BOOL)isConnected {
    NSLog(@"Current User %@ firebase", isConnected ? @"Connected To": @"Disconnected From");
}

// Use this to monitor chat partners or whoever to see if they're online

- (void) userStatusDidUpdateWithId:(NSString *)userId andStatus:(BOOL)isOnline {
    NSLog(@"%@ is currently: %@", userId, isOnline ? @"Online": @"Offline");
}

Step 3: Channel Manager

// Get Channel Manager
FSChannelManager * channelManager = fireSuite.channelManager;

// Observe Current User's Alert's Channel
[channelManager registerUserAlertsObserver:self withSelector:@selector(receivedAlert:)];

// To Send An Alert
NSMutableDictionary * alertData = [NSMutableDictionary new];
alertData[@"some"] = @"random";
alertData[@"data"] = @"here";

// Received Via
- (void) receivedAlert:(NSDictionary *)alert {
    NSString * alertType = alert[kAlertType];
    id alertData = alert[kAlertData];
    double timestamp = [alert[kAlertTimestamp] doubleValue] / 1000;
    NSDate * sentAt = [NSDate dateWithTimeIntervalSince1970:timestamp];

    NSLog(@"Received alert sentAt: %@ alertType: %@ withData: %@", sentAt, alertType, alertData);
}

// alertData can be any %@ object
[channelManager sendAlertToUserId:@"currentUserId" withAlertType:@"someAlertType" andData:alertData withCompletion:^(NSError * error) {
    if (!error) {
        NSLog(@"Alert Sent");
    }
    else {
        NSLog(@"ERROR: %@", error);
    }
}];

Step 4: Use Chat Manager

FSChatManager * chatManager = fireSuite.chatManager;

// Create A New Chat

// Set CustomId to nil for AutoId
[chatManager createNewChatForUsers:@[@"user1id", @"user2id"] withCustomId:nil andCompletionBlock:^(NSString *newChatId, NSError *error) {
    NSLog(@"Created New Chat With Id: %@", newChatId);
    [self launchNewChatSessionForChatId:newChatId];
}];

// Launch New Session
- (void) launchNewChatSessionForChatId:(NSString *)chatId {
    FireSuite * fireSuite = [FireSuite suiteManager];
    FSChatManager * chatManager = fireSuite.chatManager;
    chatManager.chatId = chatId; // chat id of new session
    chatManager.delegate = self; // who to send the messages
    chatManager.maxCount = [NSNumber numberWithInt:50]; // number of initial recent messages to receive
    [chatManager loadChatSessionWithCompletionBlock:^(NSArray *messages, NSError *error) {
        if (!error) {

            // Will receive the 50 most recent messages from firebase (maxCount)
            NSLog(@"Open with recent messages: %@", messages);
            // receivedNewMessage: will begin running now.
        }
        else {
            NSLog(@"Error: %@", error);
        }
    }];
}

// Chat Session Delegate Call
- (void) newMessageReceived:(NSMutableDictionary *)newMessage {
    NSLog(@"Received new message: %@", newMessage);
}

// To Send Message:
[chatManager sendNewMessage:@"Some message to send to the chat!"];

// To End Chat Session
- (void) endChat {
    [[FireSuite suiteManager].chatManager endChatSessionWithCompletionBlock:^{
        NSLog(@"Closed Current Chat Session");
    }];
}

Full Code Here: GitHub

\$\endgroup\$
  • \$\begingroup\$ Generally speaking, singletons should be avoided where they can. With that said, there's not enough code here to determine whether or not the singletons are avoidable. There's not much code here to actually review. I understand you've posted a link to the entire project in GitHub, but you should post the code you want the review to be focused on here and post the link only as a means of giving us some context. \$\endgroup\$ – nhgrif Mar 14 '14 at 21:16
  • \$\begingroup\$ Hi, @nhgrif, the reason I've implemented singletons in my case is so that each singleton can register observers on various levels of my app and distribute information from firebase without having to create / destroy / keep track of multiple connections. I'll reevaluate my question to see if I can explain some usage. \$\endgroup\$ – Logan Mar 14 '14 at 21:23
  • \$\begingroup\$ @nhgrif, is this a better explanation? \$\endgroup\$ – Logan Mar 14 '14 at 21:37
  • \$\begingroup\$ @Logan, I've been thinking about similar structure recently, and probably instead of class names of subsingletons (in addition to instancetype for "outer" singleton) it would be better to have id<ProtocolName> so that you can swap the whole singleton. Also, did you make any changes in this setup and/or found some bad usecases? \$\endgroup\$ – Timur Kuchkarov Mar 3 '15 at 6:34
6
\$\begingroup\$

There's very little code here to review, and I made some points in my comment to the question regarding this, however, on what is posted, here are some notes...


First of all, Xcode now autosynthesizes, so you can eliminate this line:

@synthesize firebaseURL, currentUserId, presenceManager, chatManager, channelManager;

This does mean you'll have to refer to firebaseURL as _firebaseURL instead, but this actually makes it more clear that you're referring to a private instance variable and not a method variable. Xcode while highlight it properly for you either way, which helps you distinguish, but when posted outside of Xcode, such as Github or Codereview, the highlighting isn't done, so it's easy to miss.


Second, you can slightly improve performance and save some lines of code by putting your three singletons into a collection. Once you've done that (suppose a NSDictionary, so you can use keys to determine what's what), try out this line of code:

[someArray makeObjectsPerformSelector:@selector(setUrlRefString:) 
                           withObject:firebaseURLsetter];

Third, in your getters for your singletons, you can simply write:

return [FSPresenceManager singleton];

Rather than setting a variable then returning the variable.


Fourth, you should be sure to override the setters for the singletons to be instance variables aren't created for them.

- (void)setPresenceManager {
    return;
}

Fifth, you've put a lot of work into giving yourself convenience methods for synching these three singletons. But what prevents me from doing this:

[FireSuite suiteManager].chatManager.urlRefString = @"SomethingDifferentFromTheOtherTwo";

The answer is, nothing.

You might consider making the inner singletons all private, and giving this suite manager a bunch of method wrappers.


Finally, your singleton's return type is FireSuite. I imagine you've done similar for the three inner singletons. If you ever want to subclass any of these for any reason, you're going to run into trouble. If you keep this return type, you're preventing subclassing, which may be something you want to do (see NSNumber, for example). But I don't see any particular reason for preventing subclassing.

But wait, before you switch the return type to id... hold on. If you switch to id, you can't do the following:

[FireSuite suiteManager].chatManager

Instead, you'll have to do this:

((FireSuite*)[FireSuite suiteManager].chatManager

So instead of a return type of id, we're going to want to use instancetype as our return type.

This is some Xcode magic which will assures the type you're returning is the type that's calling the method, even if you subclass. [FireSuite suiteManager] returns a FireSuite object, and [FireSuiteSubclass suiteManager] returns a FireSuiteSubclass object.


Addendum: Why does the chat manager have a delegate and methods that take block arguments?

Couldn't we have two methods to whatever protocol chat manager delegates conform to, one for success and one for error?

@protocol YourChatManagerDelegate
@required - (void)chatSessionLoadDidFailWithError:(NSError*)error;
@required - (void)chatSessionDidFinishLoadWithMessages:(NSArray*)messages;

Now our delegate simply implements these methods, and the chat manager calls these methods rather than executing the completion block which will no longer be passed.

\$\endgroup\$
  • \$\begingroup\$ Thanks, there's a lot of stuff here for me to digest and work on! How do I add instanceType? I tried to put instancetype where I would put 'id', but it is calling errors. \$\endgroup\$ – Logan Mar 14 '14 at 21:47
  • \$\begingroup\$ In both the .h and .m files, change from this: + (FireSuite *) suiteManager ........................ to this: + (instancetype) suiteManager And don't forget to check the addendum I addended. \$\endgroup\$ – nhgrif Mar 14 '14 at 21:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.