Question

I am trying to create an object model for a user and a chatroom. I'm stuck on where to place certain functionality when the objects collaborate.

At the moment all the functionality for the User is inside the User class, a snippet of its methods are:

  • User.JoinChatRoom()
  • User.WriteChatRoomMessage()
  • User.Authenticate()
  • User.JoinGroup()

I recognize that is this is a "God Object"/"Blob"" and instead we could model this as seperate objects ChatRoom, User and Group with the methods:

  • User.Authenticate()
  • ChatRoom.AddPlayer(User u)
  • ChatRoom.WriteMessage(String msg)
  • Group.AddPlayer(User u)

But I am confused about this refactor since, the way I understand object methods is that they perform an operation on the object. Therefore you can command the user to write to a chatroom, command the user to join a group, etc.

But with the "cleaner" second model this doesn't seem to fit, there is no explicit JoinChatRoom() method.

How do I design and think about what methods should be attached to an object?

Was it helpful?

Solution

In your case, you have users vs. Chatrooms. If you have methods that concern both, you could put them into either class, not much difference.

However: You will have not only users vs. Chatrooms, you will have Users vs. bills, users vs. Support requests, users vs. 100 other things. If you do things consistently by putting methods into the user class, it will explode in size.

The other problem is that whoever maintains the chatroom code probably knows better how to add a user to a chatroom then the person maintaining the user code. Adding a user is probably not trivial and will require calls to various chatroom methods that you’d prefer to be private. So you will get simpler code written by the person who knows about chatrooms if you add the method to the chatroom class.

OTHER TIPS

A user is someone who is registered and able to use the system.

A chat room is a place people can chat. What happens when a user joins a chat room? What is that thing that represents a user who has joined a chat room? That is the abstraction you are missing. Other answers are hinting at this.

You could say a user participates in a chat. You need a class that represents a user participating in a chat room. We can call it ChatRoomParticipant. What does participating in a chat require? A User and a ChatRoom.

var participant = new ChatRoomParticipant(chatRoom, user);

participant.SendMessage("Hey, everyone. I'm new here!");
participant.SendImage(File.Open(@"C:\Cat Pictures\Fluffy playing with catnip.jpg"));
participant.SendMessage("Oops. Gotta go. Someone's at my door.");
participant.LeaveChatRoom();

Now you are commanding an object to do something. Send a message. Upload a (cat) picture. Leave the chat.

Sometimes you need objects to collaborate, and it is the collaboration of those objects that needs to be modeled in its own class. Don't get too hung up on "classes must be things." You'll miss opportunities like this where the best OO design is to model the collaboration of two or more objects.

Rule of thumb: Your methods are probably in the right place if they don't need to pull data out of other objects.

This is often called "envy", as in "feature envy". If your method wants data that does not belong to it, it is basically envious of the features of some other object. Don't envy, cooperate!

Thinking about who do you want to command to do something leads to wrong designs. Object-orientation does not model the "real world" that way. In OO I can ask a piece of paper to do something, or a number to add itself. Obviously in the "real world" I can't. So don't think about it this way.

Whether you split something also comes back to the point above. If the methods in the new object get envious of the old object, or vice-versa, then the split is wrong. Don't be overly concerned about god objects or SRP. An object with a bit too many methods but a sound design is always easier to refactor then small objects but bad design.

By the looks of it you are missing a controller, an object that is calling the shots, that moves data from one object to another.

User.JoinChatRoom()

The user does not have to be an active thing that makes its own decisions. You have a UI. Someone double-clicks a room and you want the user to enter the room. You could then have the code behind the UI add the user to the room's user list without the user object being aware of it.

User.WriteChatRoomMessage()

Same here. You can have the real user (the one behind the keyboard) draft the message in the UI. Only after he clicks on the post button, you send the message to the room object, like call room.Messages.Add(text);

User should probably stick to maintaining user properties. Room should stick to managing posted messages and offering a way to obtain the message history. They do not have to be aware of their environment. You are making your problem domain objects do work outside their own scope.

Your controller object could be named ChatManager or ChatController. Initiate actions from that one and try to leave your User, Room and Group objects more passive.

It’s about flexibility.

If I tell you that methods should be located with the data they act on you’d be right to complain because the whole point of your question is what to do when the needed data isn’t all in one place. So what should you do when the data doesn’t make it obvious?

Create objects who’s methods will be stable. OOP is good at a lot of things but adding new methods to old objects is not one of them. OOP shines when you let new features come from new objects.

That’s why I prefer your second design. Because is doesn’t look like you just keep adding more and more methods to User. You’re adding new objects. That’s what OOP is good at.

I really think you are missing a ChatMessage type here, which would have let's say a Sender (that would be a user), a Channel (abstraction for chatroom, unless you want to use that term even for 1v1 conversations), and Content.

Then you also need some object to orchestrate everything, e.g. when you create a new chatroom or a new user registers, who is the source of truth for that? So let's say there is a ChatServer that has (or knows how to get from another object, if you later start breaking down the responsibilities) a list of current channels, the associations between users and channels, etc.

This would allow you to write code like

channel = server.getChannel("chatroom name");
server.addUserToChannel(channel, user);

message = User.createMessage(channel, content);
server.sendMessage(message);

Most importantly: actions that affect the server state should never be methods of other objects. This might not seem too important in trivial implementations, but let's imagine e.g. in the future you have private channels that users cannot join on their own (need to be added by another user, or invite is required).

If joining a channel is a method of User, then clearly the User class is going to become a god object. If it's a method of Channel, then the channel needs to know about possibly a lot of server state to determine if joining is allowed, going off on the path of no return where the SRP is going to not feel welcome.

If they're sufficiently leaky abstractions to get the necessary data to implement said function without implementing it as a methods/member function, then I tend to favor the independent free-standing function in languages that support it. That eliminates all thinking as to where we should put methods while strengthening encapsulation with fewer methods in the system that can access internals and potentially violate invariants.

Otherwise, I tend to favor the direction that seems to produce more desirable coupling. A common example that crops up in my field is whether an object should be able to draw itself to an (abstract) renderer or whether the renderer should be able to draw the object. There are merits both ways. Don't let people say one way is always superior to the other.

But the question should be about coupling as I see it. It revolves around who should know more. It's about the flow of knowledge. If the object has a method to draw itself, it requires a lot of knowledge of the renderer even if it's very abstract. Chances are that it will need to know about things of the renderer like how to draw lines, rectangles, images, maybe a canvas, possibly even clipping, etc. If the renderer changes its interface, it can break all code in such objects. If it's the other way around and the renderer has the functionality, it will probably need to know a lot about objects like their positions on the screen, what sort of thing they are, etc. If those objects change in their interfaces, then it breaks the renderer(s).

From my perspective, dependencies should flow towards stability (what is less likely to change in the future), since dependencies towards the unstable (things that are constantly prone to change) tend to produce cascading changes that cost (and risk) far more than they should. So it is the thing that you anticipate is more stable (or at least more confident about) in its interface that should know more about the outside world in its implementation, not the other way around, to minimize maintenance costs (i.e., the cost of changes). If the object knows how to draw itself, the renderer interface should be very stable. If the renderer instead knows how to draw these objects, then interfaces of said objects should be very stable. If you factor team members into the equation, then decoupling in their direction translates to them having to know less if they sit down and work on a piece of code. Coupling in their direction requires them to know more and have greater expertise over the architecture you designed.

One of the reasons I think many people tend to say that objects knowing how to render themselves is an anti-pattern is because there are typically fewer concrete renderers than there are objects that can be rendered. And that's worth factoring into account as to whether the renderer should know more about objects or whether the objects should know more about the renderer. But it makes little difference if the dependencies are flowing in a very stable direction. Yet I'd try to take both into account: the likelihood of changes and the cost of changes. If we have difficulty estimating the latter, then the safe bet in my opinion is to direct dependencies in the direction that has the lowest probability of requiring future changes. If we wanna build a skyscraper, we have to build on the most stable (i.e., unchanging, unmoving) foundations, or else risk seeing it collapse.

Sometimes you might want to consider conflicting forces: for example, in a chat program, the UI might want to talk to the User object only, because it shouldn't be concerned with the internal implementation, while the User object might delegate the function to the current ChatRoom object, which would be the right object to do the actual work.

This is somewhat similar to the workings of front offices and back offices in an organization: as a client, you talk to the front office person who might delegate tasks to different back offices, but you don't want or need to be concerned with that internal structure.

When struggling to find where code goes, consider whether there might be missing abstraction(s), which would be some other entity or object or actor.

Consider the consuming client code, whose usage model should be as simple as possible — dealing with few objects, rather than unnecessarily dealing with object pairs, another sign of missing abstraction (and also having simple operations rather than multiline or multi-call operations to get wrong).

Identifying missing abstractions can help make things feel more natural for the consuming client code and can also help with implementation choices.

Licensed under: CC-BY-SA with attribution
scroll top