How to design class dependency trying to avoid Law of Demeter
-
03-06-2021 - |
문제
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:
- 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
해결책
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?
다른 팁
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;