How to avoid ...Helper or ...Manager classes

Question

I have quite a few Helper classes in my project. I have read that this is a bad thing, but I suspect that "Helper" is the wrong suffix for them. I'll give an example.

First, I have a User class. I need a method GetSuggestedFriends() for a user. I want to keep the logic for determining the list of suggested friends out of the User class, so it doesn't get bloated. Right now, I have a FriendshipHelper which receives a User in its constructor. It contains the logic for getting suggested friends, and I can now call myUser.FriendshipHelper.GetSuggestedFriends().

Originally FriendshipHelper had static methods only, and a User object was passed into each one. If I was writing the class from scratch now, maybe I'd call it FriendshipManager - it also does things like adding and removing friends.

I've also read that ...Manager classes are bad, though. What should I call this class? Or, is this "bad code"? Where should the logic for getting suggested friends, current friends, and adding and removing friends live? Surely not all in a giant User class?

Answer 1

How to avoid …Helper or …Manager classes«

In general: by good Design

First, I have a User class. I need a method GetSuggestedFriends() for a user.

Yes. A user has a relationship to other users. And the relationship could be expressed as a method on user, e.g. user.isFriend(user2). This is the responsibility of the user-object. Besides that, you ask another object for help finding other friends. You delegate the responsibility for finding friends to another object and that's quite fine.

Right now, I have a FriendshipHelper which receives a User in its constructor. It contains the logic for getting suggested friends, and I can now call myUser.FriendshipHelper.GetSuggestedFriends()

That is not per se bad, but has one downside: initializing the "Helper" with one user limits the possibilities to that one user.

What you need is an object, which helps finding friends to any user. So a generic method would make sense: userMatcher.findFriendsFor(user) which in return delivers a collection of possible friends (user).

If I was writing the class from scratch now, maybe I'd call it FriendshipManager

Your problem is not writing "helper classes", it is finding the right names. ;)

it also does things like adding and removing friends.

That is a wrong design. Take yourself for example: Does your mommy add friends to your life or do you add them by yourself?

Of course the collection of friends is a property of user itself and so is the method user.addFriend(user) or user.removeFriend(user)

What should I call this class?

As said before: you only have a naming issue and your "helpers" are okay. But you have to think more carefully about the responsibilities of each object.

Where should the logic for getting suggested friends, current friends, and adding and removing friends live? Surely not all in a giant User class

No. These are two jobs for which you need one separate object, like in real life where you have people and a dating agency.

Answer 2

I would suggest that you have a FriendshipService class that has a (non-static) GetSuggestedFriends(User) method. Avoid static methods since you cannot implement an interface which makes it more difficult to test. Avoid adding the user object to the constructor since you might want to extend your FriendshipService with methods not specifically related to a single user. (For example, you might want to suggest friends to a set of users, or suggest friends based on something else)

A user should most likely not be aware of the FriendshipService (due to the Single Responsibility Pattern)