In which class would you put these methods?

Question

If I have a User class, and his account can be suspended by adding an entry to the suspensions table, which of these class/method signatures do you think is more appropriate?

User::suspend($reason, $expiryDate);
Suspension::add($userid, $reason, $expiryDate);

This is a simple example, but I have this kind of situation everywhere throughout my application. On one hand, I'd want to make it a method of the User object, since the action performed is directly related to that user object itself, but on the other hand making it a method on the suspension object seems a bit cleaner.

What do you think?

Answer 1

you suspend a user.

User.Suspend()

In your User.Suspend method, you can actually add them to your "suspension" table, or call your suspension object. This will lead to a cleaner interface since all you have to do is call the one method.

Answer 2

From these two options I would vote for Suspension::add(), if in fact this call would add an entry to the suspensions table. That way the effect that this call in the code has, in terms of the code itself (i.e. not the concepts represented by the code), would be clear: if I saw the code User::suspend(), I would expect it to modify a "suspended" flag for the User object, not modify something else in some other object.

On the other hand, in this particular instance, I think User::suspend() is more clear in general, so I would vote for it if it would mean that a suspended flag would be set for that User object, or if it would seem that way from the interface, i.e. if you wouldn't have to care where the suspension is stored since the interface of the User class would make it seem as if it's one of its properties.

Answer 3

Well if adding a suspension is the only real action, I would go with the first option and make it an action carried out by the User class.

However, if you intend on making more functionality for Suspensions, I would consider creating a class like:

class SuspensionManager
suspendUser(....)
getSuspendedUser(...)
....

Answer 4

This situation is very typical in web application design. It often becomes easier to deal with objects as being disconnected entities, as it saves you from having to retrieve objects to perform an operation for which you didn't really need the object.

The former is nicer from an OOP sense, the question is whether the performance impact of this would bother you:

User user = GetUser($userId); // unnecessary database hit?
user.suspend(reason, expiryDate);

Answer 5

Its definitely up to you. OO design is very subjective. Here, it depends on whether you view suspension as a noun (a suspension) or a verb (to suspend). If the former, it likely becomes its own object with appropriate methods and attributes. If the latter, it becomes a set of related methods and attributes of the User object.

This brings up another issue: are you a minimalist? There are those that try to keep many, light classes as opposed to a few heavy ones.

Personally, I see cohesion/coupling as outweighing all those factors by orders of magnitude. Basically, for me, it would hinge upon whether other system entities need to know about suspensions without having a User object to query with. If so, the Suspension class would be born. If not, I would keep it as a part of the User class.

Answer 6

I would be inclined to have an Account which linked the User and the Suspension

Answer 7

*This is my opinion is 100% debatable given that I don't know your entire code base/intention

I would say neither. But it really depends on how you view OOAD. I consider both User and Suspension classes have a single purpose. The User class has the responsiblity of holding information directly associated with a User (user table), and the Suspension class has the responsibility of holding information directly associated with a Suspension (suspension table). I would suggest making a UserSuspention class that has the responsibility of suspending a user.

This approach to OOAD is related to SOLID design principals. Having either the User or Suspension class be responsible for suspending a user would violate SRP (single responsibility principal)...since each class already has the responsibilty of maintaining information from their respective tables.

Your potential API may look like something below:

  public class UserSuspension
  {
    public void SuspendUser(User user, Suspension suspension) {  ...   }
    public void SuspendUser(Guid userId, string reason, DateTime expiryDate) { ... }
  }

Answer 8

It depends.

This could be one of those scenarios where there isn't a definitely right answer. It will depend on how data will move through your system, as to whether it's of more benefit to view this relationship in a data-centric, or a user-centric model.

An old rule-of-thumb is to view objects as nouns and methods as verbs, when you're trying to model things. This would tend to suggest that User is an object, and suspend is an action you might perform.

Simple ? Not really.

Someone else might argue that it made more sense to describe the suspension as an 'AccountAction', and the application of the suspension as a verb. That might lead you to a model where various subclasses of an AccountAction have an applyTo method that acts on other object types.

You may need to apply your objects to an existing database schema, in which case you'll have to take into account how your persitance layer or ORM will interact with existing record structures.

Sometimes it's down to technology. OO can be implemented in subtly different ways across different language families and this too can influence the design. Some systems favour more solid inheritance graphs, other languages emphasise more loosely interconnected objects, passing messages around.

You need to be thinking through your design in terms of how you're going to want to interact with data and state. If you think about objects, as instances of classes, representing states of data, with behaviours that you will wish to invoke, you might find the nouns and verbs pattern falling out of the sentences that you use to describe the system.

Answer 9

As others have stated, it's very subjective.

Personally, I prefer the User::suspend() alternative simply because it allows me to implement (or change the implementation of) suspension whenever I like. It leaves all the suspension logic hidden behind the User interface.

Answer 10

I often times struggle with the same problem and what I do is I ask myself if this would make sense outside of the programming world. Would you ask ,in real life , a user to suspend him/herself? Would you ask a loan application to approve itself? If the answer is no, then there needs to a specialized authority/component/service that handles that and similar scenarios. In case of loan application, the approval should best be a part of loan approval service or loan specialist. If in your case, asking a user to suspend himself makes sense in the domain you're modeling then it should belong to the user. If not then, a service that handles user account suspension and similar user account level services may be a better place.