How to design class dependency trying to avoid Law of Demeter

Question

Ok,I´ve searched and couldn´t find a suitable solution for my problem, I am redesigning a part of our point of sale system. Let´s suppose we have the following classes:

TWorkShift = class
   Date: TDateTime;
   fTotalSold: Currency;
   fSales: TList<TSale>; 
public
   property TotalSold: Currency read fTotalSold write fTotalSold;
   property Sales: Currency read fSales write fSales;
end;

TSale = class
    fAmount: Currency;
    fWorkShift: TWorkShift;
public
    property Amount: Currency read fAmount write fAmount; 
    procedure Save;  
end;

Now, the problem I am facing is trying to come to the best idea without violating the Law of Demeter. What I am trying to accomplish is the following:

1. Every time a new TSale is saved I want to add it to the Sales list of the TWorkShift of the current user, and also I want to sum the amount of the sale to the "TotalSold" of the TWorkShift.

I´ve tried two different approaches:

Approach A:

// Let´s suppose we have a working shift with the ID 1 and gets loaded from database with: CurrentShift := TWorkShift.Create(1);

NewSale := TSale.Create;
NewSale.Amount:=100;
NewSale.Save;

CurrentShift.Sales.Add(NewSale);
CurrentShift.TotalSold := CurrentShift.TotalSold + NewSale.Amount;

The problem with this approach is that It´s difficult to test, because I want to encapsulate the logic of the sum in some of the classes or somewhere else (a new class maybe?).

Approach B:

My other approach is including that code inside the TSale class itself:

procedure TSale.Save;
begin
    SaveToDataBase; 

    fWorkShift.Sales.Add(Self);
    fWorkShift.TotalSold := fWorkShift.TotalSold + Self.Amount;
end;

This approach I think violates the Law of Demeter and doesn´t feel right to me.

I Want to find a "right way" to do it maximizing code simplicity and easy of maintenance in the future. So any suggestions would be appreciated.

Thanks

Answer 1

If you want to add a sale to TWorkShift, then you should have

TWorkShift.AddSale(aSale: TSale);
begin
  Sales.Add(aSale);
end;

In other words, TWorkShift should "ask" for the thing it needs.

Also, I don't see any reason that TSale would have a TWorkShift field. A Workshift has many sales, but why would a Sale have a WorkShift?

Answer 2

You are doing something when you add items to a TList so you can use the OnNotify. I don't know if Aurelius is also using that event so I added some code for that. You only have to see if assigning the OnNotify can happen inside the framework after the list is assigned to your TWorkShift object because then it might overwrite the NotifySales event handler.

type
  TWorkShift = class
  private
    Date: TDateTime;
    fTotalSold: Currency;
    fSales: TList<TSale>;
    fNotifySales: TCollectionNotifyEvent<TSale>;
    procedure NotifySales(Sender: TObject; const Item: TSale;
      Action: TCollectionNotification);
    procedure SetSales(const Value: TList<TSale>);
  public
    property TotalSold: Currency read fTotalSold write fTotalSold;
    property Sales: TList<TSale> read fSales write SetSales;
  end;

procedure TWorkShift.NotifySales(Sender: TObject; const Item: TSale;
  Action: TCollectionNotification);
begin
  if Assigned(fNotifySales) then
    fNotifySales(Sender, Item, Action);

  case Action of
    cnAdded: fTotalSold := fTotalSold + Item.Amount;
    cnRemoved: fTotalSold := fTotalSold - Item.Amount;
  end;
end;

procedure TWorkShift.SetSales(const Value: TList<TSale>);
begin
  if Assigned(fSales) then
  begin
    fSales.OnNotify := fNotifySales;
    fNotifySales := nil;
  end;

  fSales := Value;

  if Assigned(fSales) then
  begin
    fNotifySales := fSales.OnNotify;
    fSales.OnNotify := NotifySales;
  end;
end;