Which object should have the method?

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?

Does this answer your question? [What are the practical ways to implement the SRP?](https://softwareengineering.stackexchange.com/questions/158845/what-are-the-practical-ways-to-implement-the-srp) — gnat, Oct 29 '20 at 10:37
@gnat Not quite, i understand the SRP (i think) but i don't understand how to make logical sense of the resulting methods — Richard Bamford, Oct 29 '20 at 10:43
If you're struggling with the structure of your code, consider the ways in which you'd make that code more easily testable (i.e. easier to write automated unit tests) - https://softwareengineering.stackexchange.com/questions/153410/what-are-the-design-principles-that-promote-testable-code-designing-testable-c — Ben Cottrell, Oct 29 '20 at 11:57

score 48 · Answer 1 · answered Oct 29 '20 at 19:29

48

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.

answered Oct 29 '20 at 19:29

Greg Burghardt

34,276
8
63
114

5

I like this answer because it defines a relationship between the two objects. I would use the term `connection` as it feels more accurate in this context. The user connects to the server, and that connection can involve sending and receiving any amount of messages, along with the actual connecting and disconnecting. – ndm13 Oct 30 '20 at 08:17
5

+1, best answer. There's a real disconnect between using OO to model real world objects or to model the actual problem domain. Unfortunately, most OO teaching materials keep insisting on the former, where the latter is so much more useful. – João Mendes Oct 30 '20 at 14:43
To add to your answer, such an Object is called a composition of Objects. So the ChatRoomParticipant is composed of a ChatRoom and a Participant. – John Glen Nov 01 '20 at 00:32
+1. `User.leave(chatRoom)` or `ChatRoom.kick(user)` also require validation ("is the user actually in the room?"), but a `ChatRoomParticipant.leaveChatRoom()` enforces that at a type level. – BoppreH Nov 01 '20 at 01:16
Think it like DB tables - you usually want a class for each table if you need those additional "columns" and methods. – iBug Nov 01 '20 at 16:24

Robert Bräutigam · Answer 2 · 2020-10-29T19:24:27.500

26

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.

edited Oct 29 '20 at 19:24

answered Oct 29 '20 at 11:13

Robert Bräutigam

11,473
1
17
36

Also known an Information Expert from GRASP. (https://en.m.wikipedia.org/wiki/GRASP_(object-oriented_design) – user949300 Oct 30 '20 at 08:20
3

The last two sentences should be in bold. Too many people get obsessed with SRP and write way too many classes, or way too many methods, that are only used once. You don't need to have some arbitrary limit on the size of your code if it's not providing an actual benefit outside of smaller chunks of code. – Logarr Oct 30 '20 at 18:27
But what you would say if both sides pull out the data?, ok i put the behaviour to the Chatroom, but the Chatroom pulls then out the data from the user, because it needs to fullfill its algorithm data from both,....... maybe then we have a third party, fourth and so on, The Chatroom would pull out data from all of the others. What kind of benefit i have to put then the behaviour to the Chatroom. On which metric i can decide to put it to the Chatroom, because you said placing the bevahiour to that location, which dont pull out data, but if each one of them pull out data?^^ – Robin Kreuzer Mar 16 '21 at 23:06
@RobinKreuzer You have to come up with a design in which no objects pull data out of other objects (some _rare_ exceptions apply). That is what object-orientation is at its core. Objects are there to contribute behavior. It is the key factor that makes oo more maintainable than procedural programming. – Robert Bräutigam Mar 17 '21 at 08:13
So i think this topic is so important (maybe you remember on my email about that last year, pre-last-year?). so i created a question here for it: https://softwareengineering.stackexchange.com/questions/423533/tell-one-but-ask-the-others – Robin Kreuzer Mar 18 '21 at 01:02
@RobertBräutigam *"You have to come up with a design in which no objects pull data out of other objects "* Do – Mehdi Charife Aug 15 '23 at 20:46

score 22 · Accepted Answer · edited Apr 29 '21 at 21:10

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 than 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.

Martin Maat · Answer 4 · 2020-10-30T10:45:34.010

8

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.

edited Oct 30 '20 at 10:45

answered Oct 29 '20 at 14:09

Martin Maat

18,218
3
30
57

1

+1 I agree that it seems the OP is missing an abstraction (e.g. an entity/object/actor), and that's what's making the choices awkward. – Erik Eidt Oct 29 '20 at 14:53
Strongly agree with this answer, wanted to write something similar! Chatrooms and Users will probably have distinct lifecycles and no "composition"-like relationship, where you *need* an object of the one class to create an object the other. Thus, you need a control class to manage both creating respective objects, as well as putting them into relationship to each other. Additional remark: if you want to keep track of this relationship, you could either have of a user list in the chatroom, or a reference to the chatroom(s) a user belongs to - that's a design choice depending on what you need. – kopaka Oct 30 '20 at 09:56
@Robert Bräutigam what do you think to that solution here or the similar solution from Greg Burghardt, i think thats more or less classic procedural programming, Each solution have to ask (more or less ) the collaborating classes to do it's job; we separate here the main behaviour into a Conroller/ChatRoomParticipant and anemic (ok they still have small behaviour) domain-classes – Robin Kreuzer Mar 16 '21 at 23:02
@RobertBräutigam im not sure if the highlighting did work, so i try it again with no empty space. see my last reaction in this comment-thread – Robin Kreuzer Mar 17 '21 at 13:49

candied_orange · Answer 5 · 2020-10-29T16:44:14.747

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.

score 1 · Answer 6 · answered Oct 30 '20 at 14:53

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.

score 1 · Answer 7 · 2020-10-31T21:57:42.267

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.

score 0 · Answer 8 · answered Oct 29 '20 at 13:58

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.

score 0 · Answer 9 · answered Oct 29 '20 at 15:09

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.

Which object should have the method?

9 Answers9

Linked