# Calculate a payment due after X age

I have inherited a function which calculates a first payment due after a certain age.
Using the function I typically pass in database values to custom calculate certain dates:

    Dim result = FPDA("#1/12/1952#", "9/29/1948", 1, 55)
MsgBox(result) 'in this example, I am returning first payment due after age 55


As far as the parameters, EffDate is the date the account became active, DOB is date of birth, mode is a payment mode value and age is the attained age to check. The function looks like this:

Public Function FPDA(ByVal EffDate As Date, ByVal DOB As Date, ByVal Mode As Integer, ByVal Age As Integer) As Date
'FUNCTION TO CALCULATE THE FIRST PAYMENT DUE AFTER SPECIFIED AGE.

Select Case Mode
Case "1"
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

Case "2"
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

Case "4"
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

Case "12"
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

End Select

Return EffDate

End Function


Is there a better or more efficient way than to achieve this?

-
Well, if you need to explain the meaning of the variable, I guess you could consider renaming the variables. –  Marc-Andre Feb 11 at 15:47
I would, but that is the standard for the business terminology –  Jason Bayldon Feb 11 at 15:54
The first thing I'd address is the cryptic Mode parameter. It's dying to be an Enum with real descriptive names instead of magic Integer values. And EffDate would look much cooler as FxDate, but that's just me :) –  Mat's Mug Feb 11 at 19:45
A better name for Mode could be TimeSpanType, with values like Yearly, Quarterly, Monthly and SemiAnnual. –  Mat's Mug Feb 11 at 19:48

Since the previous posts covered most of the structural issues (VB6 data structures v .NET, enumerating the payment modes, etc.), I'll address the remaining elephant in the room here. Looping through dates like this is similar to counting on your fingers to find out how many fingers you have on your hands. Nobody would even consider (or maybe "should" is the better word) doing this with a numeric type:

Dim Fingers As Integer = 1;

Do Until Fingers > 9
Fingers = Fingers + 1
Loop

Return Fingers


Much better to just calculate the result, especially if this is being looped over in the calling code. You tend to see code like the sample you posted when people are afraid to work with dates. The calculation involved isn't that hard. You first figure out the payee's birthday for the given age and find the following [insert financial vehicle here] anniversary based on the effective date. This lets you calculate the number of months until the next anniversary. Taking the modulus of the number of months until the anniversary and the number of months between payments gives you the month that the next payment is due. The only reason to work with anything other than the month is to cover cases where the birthday falls on a modal cycle:

Public Function FPDA(ByVal EffDate As DateTime, ByVal DOB As DateTime, ByVal Mode As Integer, ByVal Age As Integer) As DateTime

Dim ModeFrequency As Integer = 12 / Mode
Dim BirthDay As DateTime = DOB.AddYears(Age)
Dim TargetMonth As EffDate.Month

If BirthDay.Month > TargetMonth Then
TargetMonth = TargetMonth + 12
End If

Dim Offset As Integer = ((TargetMonth - DOB.Month) Mod ModeFrequency) + (BirthDay.Month - EffDate.Month)
DateTime NextPayment = new DateTime(BirthDay.Year, EffDate.Month, EffDate.Day).AddMonths(Offset)

'Test for DOB landing in payment month, but before or on the birthday.
If NextPayment <= BirthDay
End If

FPDA = NextPayment

End Function


Forgive any syntax errors - my VB is rusty. If you can't change the calling convention and have to work with VB6 types, there are similar date handling functions for the VB6 Date type to extract the year, month, and day. The logic would be the same though. The code above can be stream-lined a bit, TargetMonth and ModeFrequency are declared mainly for the sake of clarity.

One final note (and this is more something to be aware of as opposed to something I would change) is that the code is relying on how the language handles dates to enforce a business rule. In this case it is when the payment is due if the effective date has more days of the month than the due date. The original code (and the sample above) both assume that the due date reverts to the last day of the month (behavior of .AddMonths() and DateAdd()). Some companies assume the first of the next month. Some companies like the one I currently work for won't assign an issue date after the 28th.

-
I have accepted your answer for explaining the calculation and demonstrating the core issue with the calculation. Your concise explanation and calculation demonstration works perfectly. Thank you. –  Jason Bayldon Sep 13 at 4:04

This is going to be a brain dump -type review. Feel free to jump straight to TL;DR if my thinking out loud gets annoying :)

Public Function FPDA(ByVal EffDate As Date, ByVal DOB As Date, ByVal Mode As Integer, ByVal Age As Integer) As Date


You're taking all parameters ByVal, which is good - it means the function cannot alter these values if it tried, it's receiving a copy of what the caller gave it.

I can live with EffDate, DOB and Age identifiers. But as I've mentioned in the comments, Mode is a no-no, especially if it's a magic Integer variable.

Looking at the body of the function I think what Mode is really about is something more like this:

Public Enum TimeSpanType
Year
HalfYear
Quarter
Month
End Enum


So we can have a body like this:

Select Case Mode
Case TimeSpanType.Year
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

Case TimeSpanType.HalfYear
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

Case TimeSpanType.Quarter
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

Case TimeSpanType.Month
Do Until EffDate > DateAdd("yyyy", Age, [DOB])
Loop

End Select

Return EffDate


It's more readable and less error-prone, but it's doing the exact same thing. It gives another meaning to the received parameter EffDate, which might work now, but it's error-prone, and this is where ByVal is saving you.

I would make a copy of EffDate:

Dim result As Date
result = EffDate


And change the last line to:

Return result


Dim interval As String

Select Case Mode
Case TimeSpanType.Year
interval = "yyyy"

Case TimeSpanType.HalfYear
interval = "m"

Case TimeSpanType.Quarter
interval = "q"

Case TimeSpanType.Month
interval = "m"

End Select


And then you can just do:

Do Until result > DateAdd(interval, Age, [DOB])
Loop


But this is rather ugly. DateAdd lives in the Microsoft.VisualBasic namespace, which only exists to make VB6 developers happy with transition to the Wonderful World of .NET... but by using that namespace, you're missing out on everything the Wonderful World of .NET has to offer!

Date is just a language alias for System.DateTime, which exposes the .NET way of life - exposing methods like AddMonths, which is really all you need here:

Dim monthsToAdd As Integer

Select Case Mode
Case TimeSpanType.Year

Case TimeSpanType.HalfYear

Case TimeSpanType.Quarter

Case TimeSpanType.Month

End Select


Or better yet - get rid of the Select...Case altogether, because Enums are Integers under the hood:

Public Enum TimeSpanType
Year = 12
HalfYear = 6
Quarter = 3
Month = 1
End Enum


## TL;DR

And then you can just do this:

Dim interval As Integer
interval = CType(Mode, Integer)

Loop


And now that you've got code that's easy to read, follow and maintain, you can start thinking about how the loop could be simplified.

Strive to get rid of Imports Microsoft.VisualBasic anywhere you see it. VB.NET is .NET, not VB6. Your code will only get better.

-
Thank you, your write up was very good. I was able to stick with it the whole way. The function was UDF in access which explains the dateadd() and vb6 likeness . Why is it better to use a variable such as 'result'? Thank you for taking the time –  Jason Bayldon Feb 11 at 21:19
Because when you're reassigning EffDate, you're changing its meaning - it's no longer the parameter value you received from the caller, it's something else, and that makes it confusing code, because as you scroll down the code the meaning of the identifier changes and you have to mentally keep up with it (I've seen worse than this though). As for the naming itself, it could be personal preference, but I find an identifier named result is very explicit about its meaning. –  Mat's Mug Feb 11 at 21:27
I really like the complete process from Mode to the final Enum. This help having an incremental process to the final solution, it's more easily to reproduce. –  Marc-Andre Feb 12 at 16:10

You're doing a lot more looping than necessary. It would probably be more efficient to jump the return date to the same year than adjust the month accordingly. As was mentioned an enum, properly named makes the code much more readable and manitainable. Something like this:

Public Function FPDA(ByVal EffDate As Date, ByVal DOB As Date, ByVal PaymentMode As PaymentInterval, ByVal Age As Integer) As Date
'FUNCTION TO CALCULATE THE FIRST PAYMENT DUE AFTER SPECIFIED AGE.
'Find the year the payments is to be made
'Find the payment start day for the year in question
'These conditionals adjust the return value according to
'whether the date is before or after the birthdate and whether
'the interval is bigger than the payment interval
If FPDA < TargetDate Then
Do Until FPDA >= TargetDate
Loop
ElseIf FPDA > TargetDate Then
While FPDA.Month - TargetDate.Month >= PaymentMode
End While
End If
If FPDA.Month <> 2 AndAlso FPDA.Day <> EffDate.Day Then
FPDA = New Date(FPDA.Year, FPDA.Month, EffDate.Day)
End If
End Function

Enum PaymentInterval
Yearly = 12
Semi_Annually = 6
Quaterly = 3
Monthly = 1
End Enum


This will only loop in specific cases and then only for a few iterations.

This assumes that the payment day is the same as the EffDate day

I tried to test this with as many different scenarios as I could. If I missed something let me know.

If you would like simplicity over performance your original code could be optimized like this:

Public Function FPDA2(ByVal EffDate As Date, ByVal DOB As Date, ByVal PaymentMode As PaymentInterval, ByVal Age As Integer) As Date
FPDA2 = EffDate
Loop
If FPDA2.Month <> 2 AndAlso FPDA2.Day <> EffDate.Day Then
FPDA2 = New Date(FPDA2.Year, FPDA2.Month, EffDate.Day)
End If
End Function


Both achieve the exact same result, but one is more complicated and more efficient and the other is very simple but inefficient

-
Thanks for your help. –  Jason Bayldon Feb 11 at 21:21