5
\$\begingroup\$

Preamble

So, as most of you have likely also noticed, chat components, at least at a basic level, are becoming very common with lots of apps. I found myself tweaking and redesigning the interface from my last project all the time, so I decided to just create an easy to drop in solution. I'd love to get some second opinions on the code, the design, the class interface, or whatever else you've got to say about it!

Full Project Here

Usage

ViewController.h

#import <UIKit/UIKit.h>

#import "ChatController.h"

@interface ViewController : UIViewController <ChatControllerDelegate>

@property (strong, nonatomic) ChatController * chatController;

@end

Launch With This:

if (!_chatController) _chatController = [ChatController new];
_chatController.delegate = self;
_chatController.chatTitle = @"Simple Chat";
_chatController.opponentImg = [UIImage imageNamed:@"tempUser.png"];
[self presentViewController:_chatController animated:YES completion:nil];

Receive Message (Delegate):

- (void) sendNewMessage:(NSString *)messageString {
    NSLog(@"Received msg: %@", messageString);
}

Add Message:

It receives a dictionary with the message set as a content key. We use the dictionary within the controller so we can pass additional information like userId's, to see who sent the message, or timestamps. This is just a demo.

NSMutableDictionary * newMessageOb = [NSMutableDictionary new];
newMessageOb[kMessageContent] = @"Some Message To Send";

// Add Message Right To Collection
[_chatController addNewMessage:newMessageOb];

You'll also have to go into the controller and add some logic to assign who sent the message. Right now it's random just so you can see how it looks and works:

int sentByNumb = arc4random() % 2;
message[kMessageRuntimeSentBy] = [NSNumber numberWithInt:(sentByNumb == 0) ? kSentByOpponent : kSentByUser];

You can put whatever logic you'd want there, likely something similar to:

NSString * sentById = message[@"sentById"];
if ([sentById isEqualToString:currentUserId]) {
    message[kMessageRuntimeSentBy] = [NSNumber numberWithInt:kSentByUser];
}
else {
    message[kMessageRuntimeSentBy] = [NSNumber numberWithInt:kSentByOpponent];
}

Design

enter image description here

\$\endgroup\$
  • \$\begingroup\$ Are you asking us to review the usage code from an end-user (as a developer who might use this class) perspective? \$\endgroup\$ – nhgrif Mar 18 '14 at 11:42
  • \$\begingroup\$ @nhgrif - if interested, yes! \$\endgroup\$ – Logan Mar 18 '14 at 15:10
  • 1
    \$\begingroup\$ Be sure the constants you're using as dictionary keys are NSString * const and not #define. \$\endgroup\$ – nhgrif Mar 18 '14 at 22:45
  • \$\begingroup\$ I don't think I used any #define's in this proj. Already set up as const! \$\endgroup\$ – Logan Mar 18 '14 at 22:52
3
\$\begingroup\$

[ChatController new];

This is perfectly fine syntax, but an Objective-C purists doesn't necessarily like the new method that much. new is simply an NSObject method that looks like this:

+ (instancetype)new {
    return [[self alloc] init];
}

And as such, anything that inherits from NSObject will have this method. But the only reason it exists as far as I know is for something like a security blanket for people coming from other languages where object instantiation might look something like this:

ChatController *chatController = new ChatController();

They want to hang on to their new keyword--their clue that they're instantiating a new object. And while this is fine for those who choose to use it, a complete class would write its own factory method(s) in addition to the default one inherited from NSObject.

NSArray, NSDictionary, NSNumber, NSDate, etc, etc, etc. Most of the Foundation classes have their own method that does something similar to what the new method does. You would be wise to follow suite with a method that looks something like this:

+ (instancetype)chatController {
    return [[self alloc] init];
}

Now, someone like me who prefers doing things the real Objective-C way can do [ChatController chatController] in place of new.

As someone who might use a library like yours, the existence of such a method tells me that you've paid some attention to your instantiation methods and aren't relying on those inherited from NSObject. This assures me that any instance variables that need to be set before I call any methods (if any at all) are definitely going to be set.

As a note, if you override init, this instance variables will still be set when calling new, because remember, new is simply return [[self alloc] init]; so it calls the init method you wrote.


The fact of the matter is though, whether we use [ChatController new];, [ChatController alloc] init];, or [ChatController chatController];, your class isn't completely ready to use.

What would happen if I presented an instance of your ChatController class before I set the delegate, chatTitle, and opponentImg? Hopefully, you've written the class in such a way that nil or an empty string is a perfectly fine title, and that nil for opponentImg causes no problems. But what about the delegate?

Sometimes, objects that can have a delegate function somewhat fine without a delegate. For example, a tableview has a delegate property, but it also has a datasource property. The datasource fills the table with information. The delegate handles user interaction with the table. But if you don't need to respond to any of the user interactions and only need the table to display information to a user, your table will function just fine without a delegate. (And a table with static information doesn't need a datasource--the static cells can be completely created in IB).

Is this the case with your ChatController? Can it function just fine without a delegate?

If these three properties are all truly optional and the class works just fine if they're all set to nil, then you're fine as you are and the following information would be something optional to consider, purely a nicety (one which I always include in my classes). However, if these properties are not optional and it's not okay to set them to nil, then you really need to implement some factory methods.

Assuming only the delegate is necessary, first we'll create a new init method:

- (id)initWithDelegate:(id<ChatControllerDelegate>)delegate {
    self = [super init];
    if (self) {
        _delegate = delegate;
    }
    return self;
}

This method takes the delegate as an argument and sets this property upon while it's doing its initializing. But as is, we have to write:

ChatController *chatController = [[ChatController alloc] initWithDelegate:self];

So let's wrap it in a factory method:

+ (instancetype)chatControllerWithDelegate:(id<ChatControllerDelegate>)delegate {
    return [[self alloc] initWithDelegate:delegate];
}

Now, it's as simple as this:

ChatController *chatController = [ChatController chatControllerWithDelegate:self];

Following this same pattern, you should include a factory method that sets all the properties that a user is likely to want to set before doing anything with the object. For example:

- (id)initWithDelegate:(id<ChatControllerDelegate>)delegate
             chatTitle:(NSString*)chatTitle
         opponentImage:(UIImage*)opponentImage {
    self = [super init];
    if (self) {
        _delegate = delegate;
        _chatTitle = chatTitle;
        _opponentImage = opponentImage;
    }
    return self;
}

+ (instancetype)chatControllerWithDelegate:(id<ChatControllerDelegate>)delegate
                                 chatTitle:(NSString*)chatTitle
                             opponentImage:(NSString*)opponentImage {
    return [[self alloc] initWithDelegate:delegate
                                chatTitle:chatTitle
                            opponentImage:opponentImage];
}
| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ As far as setting properties, If you don't include an opponent image, it just orients the opponent chat bubbles to the edge. All of the properties are set to defaults in the Init call. Even without a delegate, it will run (I think I might have even put a NSLog notification). I just adopted the 'new' syntax because I liked the cleanliness of it, you're the second person to say something, so I guess I'll go back to the classic alloc]init] or a custom method as you suggest. I do like your init method suggestions overall because they make it even more clear, thanks for the suggestions! \$\endgroup\$ – Logan Mar 18 '14 at 22:33
  • \$\begingroup\$ How you personally instantiate is up to you. I'm just making sure that a factory method exists. \$\endgroup\$ – nhgrif Mar 18 '14 at 22:35
3
\$\begingroup\$

ChatControllerDelegate

First and foremost, it's a little strange to see a UIViewController subclass that has a delegate. It's not entirely unheard of however, and can be justified. But let's talk about you situation and your delegate.

- (void) sendNewMessage:(NSString *)messageString

First of all, this is a bad method name. It's confusing and unclear as to when and why this message would be called. If the ChatControllerDelegate is calling this method on its delegate when the user sends an outbound message, a better method name might be:

- (void)chatController:(ChatController*)chatController didSendNewMessage:(NSString*)message;

However, if the ChatControllerDelegate is calling this method on its delegate when a new inbound message is received, a better method name would look more like this:

- (void)chatController:(ChatController*)chatController didReceiveNewMessage:(NSString*)message;

When the ChatController object calls this method on its delegate, it's going to send self for the first argument, and the message for the second argument. The reason for doing this is similar to the reason why the Foundation classes do this. A UIViewController can delegate multiple UITableView objects. Because they send self as part of the arguments in the method call, the UIViewController can distinguish which table is calling the method and respond appropriately.

Imagine I've implemented your ChatController with a UITabBarController as the ChatControllerDelegate, and each ChatController is a different tab. Perhaps I want to automatically change tabs when the user receives a new message, or maybe just put one of the little red circles with the number in it to indicate the number of unread messages on any given tab. The UITabBarController could be delegating any number of ChatControllers, and as such, it's important that it knows which one of them the method call is coming from.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ I hadn't considered that situation with a nav controller, I'll add a self arg. There is no delegate method for receiving messages, this is only for UI. Do you suggest an alternative to delegate? I want to notify when the user inputs and tries to send a message. You could go into the chatController source code and handle it there if you want to, I just wanted to keep it simple so you could host all your chat logic somewhere and just update this as a UI element. \$\endgroup\$ – Logan Mar 18 '14 at 22:57
  • \$\begingroup\$ So, the already written ChatController will handle the sending of messages? I would definitely include the (optional) delegate method for receiving messages for the scenario I described. Are you wanting to give the delegate an opportunity to override the message being sent? \$\endgroup\$ – nhgrif Mar 18 '14 at 22:59
  • \$\begingroup\$ I don't send the messages within the controller, I have no idea what type of backend you'll be uploading the messages to. When you type and press send, it passes it to the delegate so you can upload it to your server however you like. I can't really add a delegate for receiving messages because I don't have any contact with whatever server you're using. The controller only receives messages you send it by using addMessage: \$\endgroup\$ – Logan Mar 18 '14 at 23:03
  • \$\begingroup\$ Okay, so in this case, you do only need the one delegate method. It should still include a reference to the chatController object that called it however. In this case though, I'd change the method name to something more like: - (void)chatController:(ChatController*)cc didAddNewMessage:(NSString*)message; The method is simply indicating that a new message has been added to the chat. And it matches the addNewMessage: method which you call on the ChatController to actually add a message. Your network object will receive a message, and call addNewMessage: on the chat controller... \$\endgroup\$ – nhgrif Mar 18 '14 at 23:11
  • \$\begingroup\$ ...which will in turn add the message to the view, then call chatController:didAddNewMessage: on its delegate. \$\endgroup\$ – nhgrif Mar 18 '14 at 23:11
2
\$\begingroup\$

Some things that might be missing...


Your addNewMessage: method takes an NSDictionary argument because, as you say, you want the ability to be able to add other information such as user ID, etc. This is good forward thinking.

However unfortunately, your ChatController class simply has an opponentImage property, which sets the image for messages sent by "opponent".

I would recommend adding a handful of predefined constant NSString keys that users can use in their dictionary when adding messages to affect the look in a number of ways.

  • A key for the image. Rather than an opponentImage property, we can pass in images with the dictionary for each message. This would allow chatrooms with multiple users.
  • A key for the username. Add a way to allow the controller to display chatter's names if the user wants.
  • A key for the chat bubble border color. Blue & green are nice, but you should be able to add any color the user wants, just have them pass in a UIImage constant here.
  • A key for the timestamp. You've already consider this, but reserving a predefined constant value should be important as this will effect the way the message is displayed.

You might consider some properties that the end user can easily set that will effect the overlook look and use of the chat.

Some properties to consider:

  • A UIFont property to allow the user to set the font the chat is displayed in
  • UIColor properties to set various colors on the controller
  • An NSDateFormatter property. Assuming time stamps are added, this property will allow the user to determine the format the timestamp appears in.
  • A BOOL for auto-correct and a BOOL for auto-capitalize, properties that can easily be set the behavior for the keyboard.
  • Another BOOL for timestamps on/off.

Finally, something you might consider adding would be a method to get a log of all the chat messages in the controller. The method would return an NSArray of the NSDictionary objects so the user has all the information about every message in the controller.

You might optionally also include a method for getting all messages that have timestamps occurring between two NSDate objects.

Examples:

[chatController fullChatLog];
[chatController chatLogFrom:startDate to:endDate];

These would allow the user to easily save the chat log to some sort of file format for long term storage.

| improve this answer | |
\$\endgroup\$
  • \$\begingroup\$ The borders are entirely customizable. If you set chatController.tintColor, this will change every iteration of blue for a quick fix. Or, you can go in and update whatever you want. I'm pretty sure it's opponentBubbleColor & userBubbleColor \$\endgroup\$ – Logan Mar 18 '14 at 23:30
  • \$\begingroup\$ I was looking for a good way to adapt for chat, passing an image key might be an easy way! I don't have support for names yet, I just display whoever I'm talking to on the chatTitle so it' right on top (like iMessage). \$\endgroup\$ – Logan Mar 18 '14 at 23:32
  • \$\begingroup\$ Even if you don't yet have a way for displaying names, you should go ahead and include predefined constants for several obvious things that any sort of chat system will have. Username, userid, usericon, timestamp, etc. \$\endgroup\$ – nhgrif Mar 18 '14 at 23:33
  • \$\begingroup\$ UIFont - I set one up but was getting strange results, I just haven't spent enough time. UIColor properties - Already there. Date Formatter, I'm pretty sure I have an inline called TimeStamp(). I store all of my timestamps in milliseconds since epoch, so that is what it provides. If it's not there, it should be. BOOL toggles, I'll look into implementing these. \$\endgroup\$ – Logan Mar 18 '14 at 23:34
  • \$\begingroup\$ I would also recommend allowing the addNewMessage: to send a value for the timestamp so if you're trying to load archived messages the timestamp is right, but if no timestamp argument is sent, default them all to [NSDate date] so if I'm adding in realtime, the timestamping is handled automatically by default. \$\endgroup\$ – nhgrif Mar 18 '14 at 23:34

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.