Ryfujk

I have an agreement with a customer. When the agreement ends, the customer needs to pay. The amount which the customer needs to pay increases for each month that has passed since the agreement was initiated. The maximum duration is 3 years, so I have a final expiration date. From this final expiration date, I must be able to find the periodic expiration date (the date when the payment increases). In other words, I need a function which does this:

DateTime GetNextExpirationDate(finalExpirationDate)

One of the issues which I am struggling with is that the algorithm needs to consider whether the expiration date in the current month has already passed (there will be a date each month). It also needs to consider that the day of the month for the final expiration date, might not exist for the periodic expiration dates (e.g. if the final expiration date is the 31st of something).

I have some code which is working, but it appears somewhat redundant and with many steps. I was wondering, if perhaps it could be simplified (either a little or a lot).

public DateTime GetNextExpirationDate(DateTime finalExpirationDate)

{

    int expirationDay = finalExpirationDate.Day;

    int currentMonth = DateTime.Now.Month;

    int currentYear = DateTime.Now.Year;

    int daysInCurrentMonth = DateTime.DaysInMonth(currentYear, currentMonth);



    // If the month has less days that the day of the final expiration date,

    // then we need to use the maximum date of the month.

    int expirationDayThisMonth = Math.Min(expirationDay, daysInCurrentMonth);



    DateTime expirationDateThisMonth = new DateTime(currentYear, currentMonth, expirationDayThisMonth);



    // If the periodic expiration date in the current month is already surpassed,

    // then we need to the date in the next month.

    if (expirationDateThisMonth < DateTime.Now)

    {

        DateTime dateInOneMonth = DateTime.Now.AddMonths(1);

        int nextMonthMonth = dateInOneMonth.Month;

        int nextMonthYear = dateInOneMonth.Year;

        int daysInNextMonth = DateTime.DaysInMonth(nextMonthYear, nextMonthMonth);



        int expirationDayNextMonth = Math.Min(expirationDay, daysInNextMonth);



        DateTime expirationDateNextMonth = new DateTime(nextMonthYear, nextMonthMonth, expirationDayNextMonth);



        return expirationDateNextMonth;

    }

    else

    {

        return expirationDateThisMonth;

    }            

}

Any ways to improve this?

Input/output examples:

Final expiration date: 15.12.2019

DateTime.Now: 12.03.2019

Output: 15.03.2019



Final expiration date 11.10.2019

DateTime.Now: 16.04.2019

Output: 11.05.2019

(I know that I cannot change DateTime.Now but it does work as an pseudo-input, which is why I included it).

When working with DateTime it's usually a good idea to not use a hardcoded DateTime.Now. This makes testing such methods very difficult because you cannot mock it so you have to write very tricky tests all with Now. You allso cannot easily switch to UtcNow etc. and you have to know which now it uses internally.

A much better solution would be to use another parameter like DateTime referenceDate. This way you can use any combinations of the two values and create more predictable and easier to understand tests.

Additionally I'd make this method static because it does not access any class fields etc.

Oh, I also need to add that you use excelent variable names! This is exactly how it should be done! ;-)

I would try to clean it up by extracting the tricky and repetitive day truncation logic into a reusable function. This would result in something like:

    /* Creates a DateTime instance for the given year, month and day. 

     * It will truncate the possiblyTruncatedDay down to the largest number possible 

     * for the given month in case if it's out of bounds.

     * Eg. trying to create an illegal Feb 31 instance will fallback to Feb 28 or 29) */

    private static DateTime CreateTruncating(int year, int month, int possiblyTruncatedDay)

    {

        return new DateTime(

            year,

            month,

            Math.Min(possiblyTruncatedDay, DateTime.DaysInMonth(year, month)));

    }



    public static DateTime GetNextExpirationDate(DateTime finalExpirationDate)

    {       

        int expirationDay = finalExpirationDate.Day;

        int currentMonth = DateTime.Now.Month;

        int currentYear = DateTime.Now.Year;

        DateTime expirationDateThisMonth = CreateTruncating(currentYear, currentMonth, expirationDay);



        if (expirationDateThisMonth >= DateTime.Now)

        {

           return expirationDateThisMonth;

        }



        DateTime dateInOneMonth = DateTime.Now.AddMonths(1);

        return CreateTruncating(dateInOneMonth.Year, dateInOneMonth.Month, expirationDay);

    }

Naming

Expiration vs Final Expiration seems kind of confusing, imo. Might I suggest PenaltyDate for when amount they owe increases, and ExpirationDate for that final date that they must pay.

Testability

As others have mentioned, passing in a referenceDate will allow you test more easily test outcomes.

Modularity

Breaking the redundant part into a separate function like others have suggested.

Bringing it all together

public static DateTime GetNextPenaltyDate(DateTime expirationDate, DateTime referenceDate)

{

    var penaltyDate = GetPenaltyDate(expirationDate, referenceDate);

    if (penaltyDate < referenceDate)

    {

        var nextPenaltyDate = GetPenaltyDate(expirationDate, referenceDate.AddMonths(1));

        return nextPenaltyDate;

    }

    else

    {

        return penaltyDate;

    }

}



private static DateTime GetPenaltyDate(DateTime expirationDate, DateTime referenceDate)

{

    int daysInMonth = DateTime.DaysInMonth(referenceDate.Year, referenceDate.Month);

    int penaltyDay = Math.Min(expirationDate.Day, daysInMonth);

    return new DateTime(referenceDate.Year, referenceDate.Month, penaltyDay);

}

Testing Mine via Console

class Program

{

    static void Main(string[] args)

    {

        Print(

            expirationDate: new DateTime(2019, 12, 15),

            referenceDate: new DateTime(2019, 03, 12)

        );



        Print(

            expirationDate: new DateTime(2019, 10, 11),

            referenceDate: new DateTime(2019, 04, 16)

        );



        Console.ReadKey();

    }



    private static void Print(DateTime expirationDate, DateTime referenceDate)

    {

        Console.WriteLine("Final Expiration Date: {0}", expirationDate);

        Console.WriteLine("Reference Date: {0}", referenceDate);

        Console.WriteLine("Output: {0}", Expiration.GetNextPenaltyDate(expirationDate, referenceDate));

    }

}

My Results

My output is formatted differently, but if I'm not mistaken the data matches.

Final Expiration Date: 12/15/2019 12:00:00 AM 

Reference Date: 3/12/2019 12:00:00 AM 

Output: 3/15/2019 12:00:00 AM



Final Expiration Date: 10/11/2019 12:00:00 AM

Reference Date: 4/16/2019 12:00:00 AM

Output: 5/11/2019 12:00:00 AM

Potentially simpler version?

/// <summary>

/// Number of calendar months between two DateTimes, rounding down partial months.

/// Will return x such that startDate.AddMonths(x) < endDate and

/// startDate.AddMonths(x+1) >= endDate.

/// Ignores time components.

/// </summary>

/// <param name="startDate">Start Date</param>

/// <param name="endDate">End Date</param>

/// <returns>Number of Months between Start Date and End Date</returns>

public static int MonthsBetween(DateTime startDate, DateTime endDate)

{

    return (endDate.Year - startDate.Year) * 12 + 

           (endDate.Month - startDate.Month) + 

           ((endDate.Day >= startDate.Day) ? 0 : -1); //rounding down for partial months

}





/// <summary>

/// Gets the next monthly expiration date after the Reference Date

/// with day component aligned with Final Expiration Date

/// Ignores time components.

/// </summary>

/// <param name="finalExpirationDate">Final Expiration Date</param>

/// <param name="referenceDate">Reference Date</param>

/// <returns>Next monthly expiration date after Reference Date</returns>

public DateTime GetNextExpirationDate(DateTime finalExpirationDate, DateTime referenceDate)

{

    return finalExpirationDate.AddMonths(-MonthsBetween(referenceDate, finalExpirationDate));

}

It's unclear from your description what you want to do if today is an expiration date. The code above will generate today's date in that scenario, which is what your code does. If you want to generate next month's expiration date in that scenario, change the >= to a > in MonthsBetween (and adjust the comments as appropriate, of course).