Board index » delphi » calculations on objects

calculations on objects


2005-06-15 05:23:09 PM
delphi6
Hi
The payroll application I am designing does some complicated
calculations, and I need to know from a design point, what is the best
(or better) method of implementing these calculations.
At the momement I created a TNICalc class, to calculate National
Insurance Contributions for the UK market. The calculations are done in
a set of 5 steps (automatically called by the constructor), and all
intermediate and final values, are available as properties as soon as I
have created a TNICalcs object. In the constructor, I pass in the
Employee for which the calculations must be done.
I did it this way, as I didn't believe the NI Calculations belonged
inside the TEmployee class. I also have the TNICalcs in a seperate unit
which keeps things simple and easy for DUnit testing. The same will be
done for Tax calculations.
At the moment, I am only calculating one Employee at a time, but using a
Vistor, I should be able to iterate through all Employees without any
issues.
Am I using the best design for the TEmployee and TNICalcs to work
together, or is there some design pattern I could be using to get things
working more smoothly?
Thanks for your time.
Regards
- Graeme -
 
 

Re:calculations on objects

"Graeme Geldenhuys" wrote
Quote
as I didn't believe the NI Calculations belonged
inside the TEmployee class.
Agree.
Quote
working more smoothly?
What do you regard as 'unsmooth' about the current design?
Some suggestions (that may or may not be relevant)
I don't like passing in TCustomer as a constructor param, or exposing
calculation results as properties. My tendency would be to deploy the
calculator as a singleton, and make a distinction between the calculator
itself as a service object, and any specific calculation.
Also I am not sure about binding TNICalc to TEmployee. What exactly does the
TNICalc object need to know to do its calculations?
bobD
 

Re:calculations on objects

"Bob Dawson" <XXXX@XXXXX.COM>writes
Quote
"Graeme Geldenhuys" wrote
>as I didn't believe the NI Calculations belonged
>inside the TEmployee class.

Agree.

>working more smoothly?

What do you regard as 'unsmooth' about the current design?

Some suggestions (that may or may not be relevant)

I don't like passing in TCustomer as a constructor param, or exposing
calculation results as properties. My tendency would be to deploy the
calculator as a singleton, and make a distinction between the calculator
itself as a service object, and any specific calculation.

Also I am not sure about binding TNICalc to TEmployee. What exactly does
the
TNICalc object need to know to do its calculations?
I seem to always share Bob's opinion. It must mean he's one hella smart guy!
;-)
By requiring a TEmployee object as a parameter to the constructor of
TNICalc, you are essentially forcing a new instance of TNICalc for every
employee you want to calculate. This will slow down processing as a new
TNICalc object will need to be created and destroyed for every employee.
I would also agree with Bob that de-coupling TNICalc from TEmployee as much
as possible is a good idea. TNICalc should have properties that can be set
with values required for the calculation and properties that can be read
with the results of the calculation.
 

Re:calculations on objects

Bob Dawson writes:
Quote
What do you regard as 'unsmooth' about the current design?
It has been bugging me that I need to pass in a TEmployee, but not
nearly as much as the create/destroy after looping through every employee.
Quote
Also I am not sure about binding TNICalc to TEmployee. What exactly does the
TNICalc object need to know to do its calculations?
TNICalc needs things like the Employee's NI Category, so it knows what
TNIRate (a set of percentages) to use for the calculation. Also it
needs the Employee's Basic Salary and if the Employee is a director.
I guess I can combine the things you and Scott Roberts mentioned. Create
a singleton of the TNICalc class, set the properties that are needed for
the TNICalc to work (NI Category, Basic Salary & Is Employee a director).
Then read the results as properties of the TNICalc.
Regards,
- Graeme -
 

Re:calculations on objects

Scott Roberts writes:
Quote
I seem to always share Bob's opinion. It must mean he's one hella smart guy!
;-)
It's always good knowning someone like that... :-)
Quote
By requiring a TEmployee object as a parameter to the constructor of
TNICalc, you are essentially forcing a new instance of TNICalc for every
Yeah, this is what really bugged me as well. A singleton should be a
good idea.
Quote
as possible is a good idea. TNICalc should have properties that can be set
with values required for the calculation and properties that can be read
with the results of the calculation.
Thanks for this tip, it makes a lot more sence, than passing in a
TEmployee as part of the constructor.
Regards,
- Graeme -
 

Re:calculations on objects

"Graeme Geldenhuys" <XXXX@XXXXX.COM>a écrit dans le message de news:
XXXX@XXXXX.COM...
Quote
At the momement I created a TNICalc class, to calculate National
Insurance Contributions for the UK market. The calculations are done in
a set of 5 steps (automatically called by the constructor), and all
intermediate and final values, are available as properties as soon as I
have created a TNICalcs object. In the constructor, I pass in the
Employee for which the calculations must be done.

I did it this way, as I didn't believe the NI Calculations belonged
inside the TEmployee class. I also have the TNICalcs in a seperate unit
which keeps things simple and easy for DUnit testing. The same will be
done for Tax calculations.

At the moment, I am only calculating one Employee at a time, but using a
Vistor, I should be able to iterate through all Employees without any
issues.
The Visitor pattern is definitely the one to go for, but you just need to
modify how the NICalc class is managed.
As Bob has said, you should not pass the Employee to the constructor as this
then requires a new instance for each Employee in the list.
Your NICalc class does not need to explicitly be a Singleton, although you
might never crezate more than one of them, you never know. It needs to
implement a Visit method that will be called by the Employee class and
should look something like this :
procedure TNICalc.VisitEmployee(emp: TEmployee);
begin
Reset; // reinitialises internal fields.
DoCalcStage1;
...
end;
You then need to do somthing like the following in the code that cuses the
iteration of the Employee list :
var
calc: TNICalc;
iter: TEmployeeIterator;
begin
calc := TNICalc.Create;
iter := employees.GetIterator;
while iter.Next do
begin
(iter.CurrentItem as IVisitable).Accept(calc);
// do something with calc's properties
end;
end;
So now the Employee has no idea of what is going on in the Visitor, just
that it is being visited; it is only the NICalc that knows how to handle
each Employee and you can use the iterating code to retrieve the properties
after each iteration.
Joanna
Consultant Software Engineer
TeamBUG support for UK-BUG
TeamMM support for ModelMaker
 

Re:calculations on objects

"Graeme Geldenhuys" wrote
Quote

Then read the results as properties of the TNICalc.
Certainly that would work. I was actually thinking a more stateless
calculator; something like
TNICalc = class
public
function PerformCalc(var aCalc : TNICalculation) : boolean;
end;
where
TNICalculation = record
GrossPay : currency;
BasicSalary : currency;
NIcategory : integer; //or enumeration
IsDirector : boolean;
Intermediate1 : currency;
Intermediate2 : currency;
Intermediate3 : currency;
// etc--you mentioned these before
FinalNIAmount : currency;
ResultCode : integer;
end;
So the use code looks something like
procedure TPayrollNICalcProcessor.CalcNIContributions(Emps :
TEmployeeCollection);
var
i : integer;
CalcRec : TNICalculation;
begin
for i := 0 to Emps.Count -1 do
begin
LoadCalcRec(calcRec, Emps.Employee[i]);
if FNICalc.PerformCalc(calcRec) then
// however results are used
else
// failure handling
end;
end;
This approach makes the calculator entirely independent of the rest of the
design: it knows how to do one thing, and the only thing it requires is the
call parameter (the calculation record). Similarly, the TEmployee knows
nothing about the NICalculator. The payroll processing object knows about
both and coordinates their interaction.
bobD
 

Re:calculations on objects

"Bob Dawson" <XXXX@XXXXX.COM>writes
Quote
"Graeme Geldenhuys" wrote
>
>Then read the results as properties of the TNICalc.

Certainly that would work. I was actually thinking a more stateless
calculator; something like

TNICalc = class
public
function PerformCalc(var aCalc : TNICalculation) : boolean;
end;

where

TNICalculation = record
GrossPay : currency;
BasicSalary : currency;
NIcategory : integer; //or enumeration
IsDirector : boolean;
Intermediate1 : currency;
Intermediate2 : currency;
Intermediate3 : currency;
// etc--you mentioned these before
FinalNIAmount : currency;
ResultCode : integer;
end;

So the use code looks something like

procedure TPayrollNICalcProcessor.CalcNIContributions(Emps :
TEmployeeCollection);
var
i : integer;
CalcRec : TNICalculation;
begin
for i := 0 to Emps.Count -1 do
begin
LoadCalcRec(calcRec, Emps.Employee[i]);
if FNICalc.PerformCalc(calcRec) then
// however results are used
else
// failure handling
end;
end;
I fail to see the advantage. If you change "TNICalculation = record" to
"TNICalculation = class" and add "function PerformCalc: boolean;" to it
you've got exactly what I described.
I suppose your method is slightly more efficient in memory usage in that you
have a thread-safe singleton while having a single class would require a
TNICalculation object for each thread. However, having a single class with
properties allows you to set the "result properties" to read-only. I suppose
the difference is trivial either way.
 

Re:calculations on objects

"Jim Cooper" <XXXX@XXXXX.COM>writes
Quote
No, but OTOH an NICalc class should not need state either, so I would
make the calc method a class function.
Just curious, but why?
 

Re:calculations on objects

Quote
The Visitor pattern is definitely the one to go for
I'm not so convinced by that. Unless you have a strange network of
employees which is difficult to navigate, e.g. where employees can
contain other employees, I wouldn't have thought it was necessary.
Also, it is almost certain that there will be times you want to do an NI
calculation on only one employee (even in a pay run you will be handling
only one employee at a time since there are other calculations to make
than NI).
Quote
As Bob has said, you should not pass the Employee to the constructor as this
then requires a new instance for each Employee in the list.
No need for the employee as a constructor parameter, but it is perfectly
fine as a parameter to the NI calculation method.
Quote
Your NICalc class does not need to explicitly be a Singleton
No, but OTOH an NICalc class should not need state either, so I would
make the calc method a class function.
Quote
You then need to do somthing like the following in the code that cuses the
iteration of the Employee list :
Why not just do :
function TNationalInsurance.Calculate(Employee : TEmployee)
begin
Result := ....;
end;
Then using an iterator is fine should it be necessary :
var
iter: TEmployeeIterator;
begin
iter := employees.GetIterator;
while iter.Next do
begin
TNationalInsurance.Calculate(iter.CurrentItem);
...
end;
end;
Otherwise you are mixing two patterns where one will do.
Cheers,
Jim Cooper
__________________________________________
Jim Cooper XXXX@XXXXX.COM
Tabdee Ltd www.tabdee.ltd.uk
TurboSync - Connecting Delphi to your Palm
__________________________________________
 

Re:calculations on objects

"Scott Roberts" wrote
Quote

I fail to see the advantage. If you change "TNICalculation = record" to
"TNICalculation = class" and add "function PerformCalc: boolean;" to it
you've got exactly what I described.
Yes, the difference is perhaps stylistic. I think that setting up a stateful
service object that requires many properties to be set leads to it being
called improperly (opps, forgot to set property 'x'), whereas is the input
data is consolidated as a record then it is harder to forget to set some of
the input params.
It also lets you separate out the engine lifetime issue: changing TNICalc
from a normal object to a singleton to a service manager controlling an
array of internal calculators doesn't affect the call.
Quote
the difference is trivial either way.
Certainly either will work, as would Joanna's visitor approach. Choice
depends on style and relative code priorities (speed, footprint, probable
evolution direction).
bobD
 

Re:calculations on objects

"Bob Dawson" <XXXX@XXXXX.COM>writes
Quote
Yes, the difference is perhaps stylistic. I think that setting up a
stateful
service object that requires many properties to be set leads to it being
called improperly (opps, forgot to set property 'x'), whereas is the input
data is consolidated as a record then it is harder to forget to set some of
the input params.
I was thinking just the opposite. When the parameters are defined in an
entirely different place (record vs class properties) one would be more
prone to set the parameters incorrectly. There is an intrinsic coupling
between an object's methods and it is properties. I think this type of
coupling mirrors perfectly a calculation routine and the parameters
necessary to perform the calculation.
As you said, it is mostly a matter of taste.
 

Re:calculations on objects

"Jim Cooper" wrote
Quote

No, but OTOH an NICalc class should not need state either, so
I would make the calc method a class function.
Depends on whether the calculator needs to do some reads to initialize its
internal calculations: rate tables, etc. Parameterization of the underlying
calcuation would argue against a class function.
bobD
 

Re:calculations on objects

Jim Cooper writes:
Quote
Also, it is almost certain that there will be times you want to do an NI
calculation on only one employee (even in a pay run you will be handling
100% correct. There will be times when I need to do a single Employee
run. I am using the tiOPF framework, which already has Visitor
capabilities built into the base object all my business objects inherit
from.
tiOPF is built on the Visitor pattern, so it fits well into the
frameworks design. I think I will start with a visitor for the monthly
payroll run, and then also design some option (maybe another visitor) to
allow a single Employee run.
Thinking about it, a Visitor makes a lot of sense, and I can do a couple
of checks in the Accept() method, to decide of I need to calculate NI
for that Employee or not.
e.g.: only visit the Employee if the Employee is not on hold, or if the
Employee is not in dispute, etc..
Quote
No need for the employee as a constructor parameter, but it is perfectly
fine as a parameter to the NI calculation method.
Everybody agrees on this one, so that is definitely going! :-)
Quote
No, but OTOH an NICalc class should not need state either, so I would
make the calc method a class function.
The NICalc class would need to setup some internal values (lookup Rates
based on the NI Category, etc..), so wouldn't that interfere with the
workings of a class function?
Regards,
- Graeme -
 

Re:calculations on objects

Quote
Otherwise you are mixing two patterns where one will do.
agreed
I often see such designs and wonder 'why ?' ..
and I thought I complicated stuff :D
I'd also use the same approach using Calc methods