PriceComparisonStandardsGroup / AddOns

7 stars 9 forks source link

missing finance calculation examples #4

Closed davie closed 8 years ago

davie commented 8 years ago

At the meeting last week it was mentioned that there was a spreadsheet with the calculations & examples - could someone add these?

wardpatrick commented 8 years ago

Dave Fox has asked @StephenCollis to look this over, so it is happening.

InstalmentCalculator.xlsx

StephenCollis commented 8 years ago

Please find the initial draft of the installment calculator pseudo code. It may benefit from additional commenting. This code may be executed on the .net fiddle sandbox InstallmentCalculator.cs.txt

wardpatrick commented 8 years ago

Take it SetupFee is the 'The Sum of all Fees that cannot be added to the loan' and LoanSetupFee 'Sum of all Fees that are added to the loan'

StephenCollis commented 8 years ago

The "Setup fee" is the broker fee added to the premium for the policy setup regardless of whether the policy if financed or not, the "LoanSetupFee" is an optional additional fee that the broker/Insurer may charge for the privilege of setting up the finance.

Thinking about it I suspect that the "Setup Fee" is noise to an aggregator as it would already have been included in the price returned.

Stephen Collis

IT Director Tel 01473 542001 Mob 07748 633867

On 14 June 2016 at 08:31, Patrick Ward notifications@github.com wrote:

Take it SetupFee is the 'The Sum of all Fees that cannot be added to the loan' and LoanSetupFee 'Sum of all Fees that are added to the loan'

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PriceComparisonStandardsGroup/AddOns/issues/4#issuecomment-225801530, or mute the thread https://github.com/notifications/unsubscribe/APg4IwdUc1ede0QSKsEKESzZWvh2aDCZks5qLli3gaJpZM4I0Vlk .

wardpatrick commented 8 years ago

@StephenCollis, @apstewart can you take a look at some queries I have added to the code snippet. Theme focuses around the fact fees and deposits could be denoted as a percentage amount and/or a fixed amount. @apstewart covered some scenarios, that is why we have an AmountType enum. Also would be useful to explicitly bind the public props in the pseudocode to the schema properties so we 100% clear on what is getting passed in.

ProgramQuestions.cs.txt

DaveFoxOpenGI commented 8 years ago

The setup fee is NOT a fee regardless of whether finance is available or not. It is a fee for taking out finance but the provider wants the fee paid up front. The finance fee is a fee for taking out finance but is spread out over the term of the loan (and as such incurs interest charges also). Other fees that are regardless of finance are already included in the main insurance premium.

wardpatrick commented 8 years ago

So in the schema we denote a SetupFee, but we don't permit FinanceFee, so @DaveFoxOpenGI you suggest we add this.

In the Pseudo code we have LoanSetupFees and SetupFees. So LoanSetupFees (Pseudo code) = SetupFee (schema), and SetupFee (Pseudo code) = FinanceFee (schema).

So to remedy this we need to ensure that the Fees are clearly typed, SetupFee must not be added to the loan, AddToLoan="false". FinanceFee AddToLoan="true"

@apstewart, @StephenCollis do you agree with this?

StephenCollis commented 8 years ago

Sorry my miss-understanding I will make the corrections to the code. and make sure the variables names in the sudo code are in line with the schema, about to go into another meeting so give me a couple of hours.

Stephen Collis

IT Director Tel 01473 542001 Mob 07748 633867

On 14 June 2016 at 11:22, DaveFoxOpenGI notifications@github.com wrote:

The setup fee is NOT a fee regardless of whether finance is available or not. It is a fee for taking out finance but the provider wants the fee paid up front. The finance fee is a fee for taking out finance but is spread out over the term of the loan (and as such insurs interest charges also). Other fees that are regardless of finance are already included in the main insurance premium.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PriceComparisonStandardsGroup/AddOns/issues/4#issuecomment-225840731, or mute the thread https://github.com/notifications/unsubscribe/APg4I7M5_SUwFBAPzLzt0kW92LFfm_-eks5qLoD6gaJpZM4I0Vlk .

wardpatrick commented 8 years ago

Gents, do we need to change the schema like this?

<c:Tier Min="0.00" Plan="Standard" Max="0.00">
        <c:Finance Cap="0.00" Term="1" Instalments="2"/>
        <c:Deposit Min="0.00" AmountType="Percentage">0.00</c:Deposit>
        <c:Interest Percentage="0.00" minimumAmount="0.00"/>
        <c:Charges>
            <c:Charge xsi:type="c:SetupFee">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
            <c:Charge xsi:type="c:FinanceFee">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
        </c:Charges>
    </c:Tier>
DaveFoxOpenGI commented 8 years ago

Yes, the above looks good to me.

wardpatrick commented 8 years ago

@StephenCollis, @apstewart can you work with that if we change the charges section?

apstewart commented 8 years ago

I believe the name of the charges should be at the discretion of the providers. In the schema currently we have an attribute 'addToLoan' on the charge element, this attribute is used to identify if a charge is part of the initial payment or is included in interest calculations.

wardpatrick commented 8 years ago

@apstewart way means that we would still need to change the Charge name to an xs:token, currently it is restricted to an enum.

That works as all @StephenCollis need to do is to class any addToLoan fees as Finance fees, and likewise any non-addToLoan as setup fees as defined by @DaveFoxOpenGI , but understand we may need to be a bit more verbose with the terminology. instead of addToLoan we could denote a FeeType as Setup or Finance, of just stick with the boolean, what say you Gents @StephenCollis @DaveFoxOpenGI

e.g.

    <c:Tier Min="0.00" Plan="Standard" Max="0.00">
        <c:Finance Cap="0.00" Term="1" Instalments="2"/>
        <c:Deposit Min="0.00" AmountType="Percentage">0.00</c:Deposit>
        <c:Interest Percentage="0.00" minimumAmount="0.00"/>
        <c:Charges>
            <c:Charge FeeType="SetUp" Name="Protection Racket Payment">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
                        <c:Charge FeeType="Finance" Name="Paperwork Fee">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
        </c:Charges>
    </c:Tier>
apstewart commented 8 years ago

So we have three options, two from @wardpatrick and this one.

<c:Tier Min="0.00" Plan="Standard" Max="0.00">
    <c:Finance Cap="0.00" Term="1" Instalments="2"/>
    <c:Deposit Min="0.00" AmountType="Percentage">0.00</c:Deposit>
    <c:Interest Percentage="0.00" minimumAmount="0.00"/>
    <c:Charges>
        <c:Charge Name="Protection Racket Payment" AddToLoan="False">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
        <c:Charge Name="Paperwork Fee" AddToLoan="True">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
    </c:Charges>
</c:Tier>
wardpatrick commented 8 years ago

I agree we need to at least drop the restriction on Charge Name but after that not fussy on the chosen option as we can work with all of them.

@apstewart take it you vouch for option 3 i.e.

  <c:Charge Name="Protection Racket Payment" AddToLoan="False">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
DaveFoxOpenGI commented 8 years ago

After discussion with Stephen Collis we'd like to suggest the charges are defined thus - due to there being potentially three points a Charge (due to paying by instalments) might be added during the calculation. Also if we need more charges to be applied at different calculation points this is extensible by adding another 'Type'.

Also that only one Charge node should be sent per Type and a Type can only have one Amount node. E.g.

<c:Tier Min="0.00" Plan="Standard" Max="0.00">
    <c:Finance Cap="0.00" Term="1" Instalments="2"/>
    <c:Deposit Min="0.00" AmountType="Percentage">0.00</c:Deposit>
    <c:Interest Percentage="0.00" minimumAmount="0.00"/>
    <c:Charges>
        <c:Charge xsi:type="c:AppliedToDeposit">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
        <c:Charge xsi:type="c:AppliedToLoan">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
        <c:Charge xsi:type="c:AppliedToPremium">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
    </c:Charges>
</c:Tier>
wardpatrick commented 8 years ago

This would keep all parties happy, Name being optional free text (xs:token). Not relevant for calculation logic but good for audit trail. @apstewart you happy with this?

<c:Tier Min="0.00" Plan="Standard" Max="0.00">
    <c:Finance Cap="0.00" Term="1" Instalments="2"/>
    <c:Deposit Min="0.00" AmountType="Percentage">0.00</c:Deposit>
    <c:Interest Percentage="0.00" minimumAmount="0.00"/>
    <c:Charges>
        <c:Charge xsi:type="c:AppliedToDeposit" Name="Protection Racket Payment">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
        <c:Charge xsi:type="c:AppliedToLoan" Name="Paperwork Fee">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
        <c:Charge xsi:type="c:AppliedToPremium" Name="Stealth Tax">
            <c:Amount AmountType="Percentage">0.00</c:Amount>
        </c:Charge>
    </c:Charges>
</c:Tier> 
apstewart commented 8 years ago

That works for me, it makes the intent explicit. I'm not sure the name works in this context though as there might be multiple charges combined. I expect in most cases it will be a single charge, but there is always one...

wardpatrick commented 8 years ago

OK, give me five an will update schema accordingly

wardpatrick commented 8 years ago

Right then, schema now validates the following, and only allows one charge per charge type.

<c:Tier Min="0.00" Plan="Standard" Max="0.00">
        <c:Finance Cap="0.00" Term="1" Instalments="2"/>
        <c:Deposit Min="0.00" AmountType="Percentage">0.00</c:Deposit>
        <c:Interest Percentage="0.00" minimumAmount="0.00"/>
        <c:Charges>
            <c:Charge xsi:type="c:AppliedToLoan" Name="Setup Fee">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
            <c:Charge xsi:type="c:AppliedToDeposit" Name="Peanuts Fee">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
            <c:Charge xsi:type="c:AppliedToPremium">
                <c:Amount AmountType="Percentage">0.00</c:Amount>
            </c:Charge>
        </c:Charges>
    </c:Tier>

Once @StephenCollis provides updated Pseudo we should be in a position to close this issue.

StephenCollis commented 8 years ago

Pleased find attached revised sample code InstallmentCalculator.cs.txt

wardpatrick commented 8 years ago

Are we not missing this in the Pseudo code? i.e. the min interest check.

            decimal minInterestAmount = 5m; 

            decimal interestAmount = loanAmount*NetInterestRate;

            if minInterestAmount < interestAmount
            {
               interestAmount = minInterestAmount;
            }

            decimal installmentAmount = (loanAmount + interestAmount)/NoOfInstallments;
StephenCollis commented 8 years ago

I have now included this with the MAX function decimal interestAmount = Math.Max(MinInterestAmount,loanAmount * NetInterestRate); InstallmentCalculator.cs.txt

StephenCollis commented 8 years ago

Are we happy to close this now, I don't think I have permission to upload the pseudo code file into the repository?

apstewart commented 8 years ago

I'll convert to markdown and add to the repo.

wardpatrick commented 8 years ago

We need top make sure we include this on this level plan, that also had a min interest check

StephenCollis commented 8 years ago

It can be added to the level plan but I don't think it is needed as this can be easily covered by the MinLoanAmount.

Stephen Collis

IT Director Tel 01473 542001 Mob 07748 633867

On 15 June 2016 at 08:17, Patrick Ward notifications@github.com wrote:

WE need top make sure we include this on this level plan, that also had a min interest check

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/PriceComparisonStandardsGroup/AddOns/issues/4#issuecomment-226107476, or mute the thread https://github.com/notifications/unsubscribe/APg4I4YBt2WITo5H4mv_Y95UnQzNp9BQks5qL6cZgaJpZM4I0Vlk .

apstewart commented 8 years ago

I've submitted a pull request with the change to include the min interest check.

The Min interest check will still be needed in cases where a low MinLoanAmount exists, I don't see the two things as being exclusive.

If everyone is happy I will merge the change and close this issue.

StephenCollis commented 8 years ago

Yes I do see your point and you are correct, I am happy with the change.

wardpatrick commented 8 years ago

Look good to me, happy for @apstewart to merge and close