Here is some code calling a method:
Client c = LoadClient();
BonusHistory bonus = bhService.GetBonusHistory(DateTimeService.Now.Year, DateTimeService.Now.Month, c.Id);
and here is an interesting part of that method:
public BonusHistory GetBonusHistory ( int year, int month, int clientId )
{
//...........................
BonusHistory bh = null;
bh = query.UniqueResult<BonusHistory> ();
if (bh == null)
{
bh = new BonusHistory ();
Client client = session.Load<Client> (clientId);
bh.Client = client;
//........................
}
return bh;
}
Do you see anything wrong here?
The caller already has the Client entity, but it sends only its id to the GetBonusHistory method. At one point, GetBonusHistory needs the Client entity in order to create a new BonusHistory (which should have been done through a factory, but that's another story). The method doesn't have other choice but to load the entity again from the database, and this is not cool. The caller is a bad citizen because it does not help the classes with which it communicates.
This type of problem was easy to spot here, but with other occasions it can be much more subtle and it can create real (performance) problems.
Conclusion: Don't send entity id's in method calls because sooner or later they will have to do unnecessary calls to the database. Instead, use objects which have a conceptual meaning in that message.