Most logically consistent way to create methods of the form a affects b?

Question

I have a common scenario where I have an object (a) that logically affects another object (b), like a makes a note on b, or a marks b as disabled (usually a is a user object of the system and b is some kind of business object like a customer or an appointment).

In the past, the method was on the affected object like:

customer.Disable(user); // user is marking the customer as disabled

... but it doesn't read well (it appears backwards). The opposite:

user.Disable(customer);

... reads better, but there's something about it that I can't put my finger on that just doesn't seem right. The only other option is to have a static intermediary:

ThirdPartyClass.DisableCustomer(customer, user);

... but the domain objects end up becoming more like DTOs and I prefer the domain model approach because it makes more sense.

So the question is: which makes the most sense? Is there an option I haven't considered? Who actually owns an operation like this?

Edit

With the second option, the actionable objects in the system (users usually) end up becoming huge because the users of the system are the ones that pretty much do and affect everything.

Answer 1

How about

customer.SetDisabledBy(user);

or if using C# 4.0 or a different language with similar capabilities:

customer.SetDisabled(by: user);

or if you're using C# 3.5 or newer, you can write the following:

user.DisableCustomer(customer);

while having the DisableCustomer method be an extension method sitting in a class called CustomerActions which sits together with the Customer class. That way, when you're using the Customer namespace, User will have that extension method. When you're not using it, it's gone. User is no longer a god object but the intermediary class is nowhere to be found in the code. You might also have an OrderActions class which will provide user.CancelOrder(order), etc.

Good question, made me think!

Answer 2

user.Disable(customer) almost seems like it's saying that user is being disabled, since user.Disable() would read like it's acting on the user, not the customer.

user.DisableCustomer(customer) reads a little better, I think, as it makes it clear that the customer is being disabled. It also means you don't need an extra class just for the sake of it.

Answer 3

If your user objects are taking on so many responsibilities that it feels like they have too many "user.DoFooOnBar(bar)" methods, then you may be looking at a god object. You might feel better by either using interfaces to break up the roles of user into domain-specific pieces, or using composition to do the same.

Interfaces

You'd have an ICustomerController interface that User implements, then when you need the users to disable customers, you cast the customer into an ICustomerControl and can use ICC.DisableCustomer(customer). That provides a move natural reading because the customer is being manipulated, not by a user, but by "something which manipulates customers."

It also leaves you more flexible to let things that aren't users update customers, if you needed to.

Composition

Just an extension from the last idea: instead of users being ICustomerControllers (IS-A) they would own an ICustomerController (HAS-A). So now the call reads as user.CustomerController.Disable(customer). Again, this makes it clear that users are acting in a certain capacity (to affect customers) and only against that one domain of objects (customers).

You hadn't mentioned that problem, but a method like user.Disable() could seem to affect anything, even the user itself. Either of the above corrects that.

Answer 4

As you point out having lots of methods on the User class is introducing the God Object anti-pattern. It seems that the problem is one of encapsulation, so the data flow

User performs an action on an entity

can be re-interpreted as

System perform command on entity with reference to User

This would mean a design that favoured using a Command pattern with a typical command being like this:

public class DisableCustomerCommand implements EntityCommand<Customer,User> {

  // This is the single EntityCommand templated method
  public Customer execute(Customer customer, User user) {
    customer.setEnabled(false);

    // TODO Add in audit code for User object

    return customer;
  }
}

This follows the general OOD principle of creating find grained simple objects that do very little, but do it well.

Edit

I should point out that this is an over-simplification. Typically, the disabling of a Customer would require several other fields to be modified. Also this does lead to anaemic entities (Customer would be just a bunch of getters/setters), but sometimes that can be a simpler overall design.