Verb Subject

(DeleteMe: tentative & sketchy description. Enrich, delete or refactor at will.)

An OO CodeSmell. A method name matches the "VerbSubject" pattern, and is passed an instance of the Subject.

  public class Manager {
// ...
public void sendOrder(Order anOrder) {
// ...
}
  }

This usually calls for MoveMethod, resulting in :

  public class Manager {
// ...
public void sendOrder(Order anOrder /*, extra params */) {
Order.send();
}
  }

  public class Order {
// ...
public void send(/* extra params */) {
// ...
}
  }

Precursor refactorings : IntroduceParameterObject?

This is only a special case of the LawOfDemeter.

(DeleteMe: When it makes sense. I would try making it make sense, refactoring the page, but I have no idea what is trying to be said.)

I hate to be a stick in the mud, but this example makes no sense to me; AT ALL.

We asked the manager to send an order with extra params (ChilliSauce??). Presumably, the Manager had something to do with this process. After refactoring, the manager calls the Order and passes it the extra params but NOT itself. Apparently the functionality was implementable with NO reference to the manager object at all. Not only that, but we have duplicated the function signature of Send in both the Order and the manager. This is not OnceAndOnlyOnce. Any change to the list of extra parameters now requires two changes in two places.

In C++, I would refactor the sendOrder(const Order &) to send(const Order &). The well-named variable parameter will clarify usage by the caller and the declaration is still clear. Overloading will distinguish send(const Order &) from send(const Bill &).

Why is send a member of Manager? In the context, I am imagining Order contains the definition of just the Order, whereas Manager understands whether we send these by fax or post or email. Manager is responsible for the transmission of the Order. Making order responsible for sending itself, IMHO smells.

-- Confused

The above example makes sense because of the following:

Send() has nothing to do with Manager, it has do to with Order, that is why it doesn't belong in the Manager class.
The above refactoring is just step one: step two is finding all the code that calls Manager.sendOrder() and replacing it with Order.send(). Step three is then removing Manager.sendOrder() altogether.
Manager may still need a call to Order.send(), but that doesn't mean we should have the code in Manager. Order may not be responsible for sending itself, but it should know how to send itself.

The other advantage is that method names are more concise and clear.

This is more clearly explained in OnceAndOnlyOnce and OneResponsibilityRule.

Does this make you less Confused? If so, we can ReFactor this page and remove the discussion. --MichaelRichards

I think we have two issues here, muddled by a poorly-chosen example. I think the best illustration of VerbSubject smell is the simple case where

 Order feed_me (pizza, mushnik);
 manager.sendOrder (feed_me);

should eliminate the known Subject type and leave just the Verb:

 Order feed_me (pizza, mushnik);
 manager.send (feed_me);

Whether "sending" is best handled by Manager or Order is really an illustration of the MoveMethod deodorizer, and probably doesn't belong here. Since there is no C/J style sample on that page, I propose adapting this sample like so:

  public class Manager {
public void send(Order anOrder, Food whichFood, Person fromWhom) {
anOrder.ready( whichFood );
anOrder.aim( fromWhom );
anOrder.send( fromWhom.FavoriteTarget?() );
  }

I would expect Order to know which food, and which person; more above the same with pizza. - btw.: what is mushnik? - --sWagner --

becomes (recalling that Refactoring preserves existing interfaces):

  public class Manager {
public void send(Order anOrder, Food whichFood, Person fromWhom) {
Order.send (whichFood, fromWhom);
}
  }
  public class Order {
public void send(Food whichFood, Person fromWhom) {
ready( whichFood );
aim( fromWhom );
send( fromWhom.FavoriteTarget?() );
}
  }

Do feel free to slash my example to ribbons. As a newcomer to this forum, I'm readily prepared to have gotten a few points wrong. --SeanGugler?

But what if the managers task is wider than simply sending the order? E.g.

  public class Manager {
public void sendOrder(Order order) {
addToOpenOrders(order);
order.send();
notifyAccountant(order);
}
  }

A fine counter-example. The point of the VerbSubject and MoveMethod pages is to illustrate appropriate contexts for applying those deodorizing patterns -- why not demonstrate an INappropriate context as well? Though I recommend renaming mine as DumbManager and yours as SmartManager for clarity. --SeanGugler?

I can only subscribe to your proposal to illustrate INappropriate contexts of patterns. Since the design patterns became so popular, I have seen sources so overpatterned just for sake of patterns. DoTheMostCoolThingYouHaveJustReadAboutThatMightWork? :-( --GregorRayman

If the manager's task is wider than simply sending the order, then the method should probably not be named simply sendOrder(). It would no longer be a MeaningfulName. --ChristopherBennage?