HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpModerate

Determining expiration date based on list of months in which to refresh

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
expirationdeterminingrefreshdatemonthsbasedwhichlist

Problem

I have a list of months in which users are required to update some data. I want to determine at what date they should have updated their data. They must update in March, May and August. If today is March, I want to get back the 1st of March. If today is February, I want to get back last year's August. In reality the list wouldn't be hardcoded.

I do have some working code, but I wonder if there is a simpler / more elegant of writing this (the GetCheckDate method).

using System;
using System.Collections.Generic;
using System.Linq;

internal class Program
{
    private static void Main()
    {
        var listOfMonths = new List {3, 5, 8};
        Console.WriteLine(GetCheckDate(listOfMonths,
            new DateTime(2014, 2, 1))); // expect 2013-08-01
        Console.WriteLine(GetCheckDate(listOfMonths,
            new DateTime(2014, 3, 1))); // expect 2014-03-01
        Console.WriteLine(GetCheckDate(listOfMonths,
            new DateTime(2014, 7, 1))); // expect 2014-05-01
        Console.WriteLine(GetCheckDate(null,
            new DateTime(2014, 7, 1))); // expect 2013-07-1
        Console.WriteLine(GetCheckDate(new List(),
            new DateTime(2014, 7, 1))); // expect 2013-07-1
    }

    private static DateTime GetCheckDate(IEnumerable listOfMonths,
        DateTime curDate)
    {
        DateTime result = curDate.AddYears(-1);
        if (listOfMonths == null) return result;
        listOfMonths = listOfMonths.ToList();
        if (!listOfMonths.Any()) return result;

        listOfMonths = listOfMonths.OrderByDescending(m => m).ToList();
        var greatestSmallerThan =
            listOfMonths.FirstOrDefault(m => m  m > curDate.Month);
        result = new DateTime(
            greatestSmallerThan.Value <= curDate.Month
                ? curDate.Year : curDate.Year - 1, greatestSmallerThan.Value, 1);

        return result;
    }
}

Solution

I suggest you to improve naming:

  • months instead of listOfMonths because its not list, and its better not to include variable type description into name



  • date instead of curDate - its better to avoid abbreviations, also current method should not care whether moth is current or not - it just search previous month from given months



  • Possibly I would rename method to something like GetPreviousMonth



Also I don't see any need to have nullable type for months sequence. It should have integers. If month exist in list, then you can avoid sorting - so do this check first. Thus you are not going to add/remove month, then saving ordered sequence to array is little better:

public static DateTime GetCheckDate(IEnumerable months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.AddDays(1 - date.Day);

    var orderedMonths = months.OrderByDescending(m => m).ToArray();
    if (date.Month > orderedMonths.First())
        return new DateTime(date.Year, orderedMonths.First(), 1);

    if (date.Month  m < date.Month), 1);
}


But I also prefer readability on first place (performance should be improved only if its a problem) so I would go with Max and Min month instead of sorting them and keeping in mind what is Last and what is First:

public static DateTime GetCheckDate(IEnumerable months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))          
        return date.AddDays(1 - date.Day);            

    if (months.Max()  m < date.Month).Max(), 1);
}


If I would continue making it more readable, then following extension methods would be useful:

public static DateTime FirstDayOfMonth(this DateTime date, int month = 0)
{
    return new DateTime(date.Year, month == 0 ? date.Month : month, 1);
}

public static DateTime PreviousYear(this DateTime date)
{
    return date.AddYears(-1);
}


Now whole method is very easy to understand:

public static DateTime GetCheckDate(IEnumerable months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.FirstDayOfMonth();

    if (months.Max()  m < date.Month).Max());
}

Code Snippets

public static DateTime GetCheckDate(IEnumerable<int> months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.AddDays(1 - date.Day);

    var orderedMonths = months.OrderByDescending(m => m).ToArray();
    if (date.Month > orderedMonths.First())
        return new DateTime(date.Year, orderedMonths.First(), 1);

    if (date.Month < orderedMonths.Last())            
        return new DateTime(date.Year - 1, orderedMonths.First(), 1);

    return new DateTime(date.Year, orderedMonths.First(m => m < date.Month), 1);
}
public static DateTime GetCheckDate(IEnumerable<int> months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))          
        return date.AddDays(1 - date.Day);            

    if (months.Max() < date.Month)
        return new DateTime(date.Year, months.Max(), 1);

    if (date.Month < months.Min())            
        return new DateTime(date.Year - 1, months.Max(), 1);

    return new DateTime(date.Year, months.Where(m => m < date.Month).Max(), 1);
}
public static DateTime FirstDayOfMonth(this DateTime date, int month = 0)
{
    return new DateTime(date.Year, month == 0 ? date.Month : month, 1);
}

public static DateTime PreviousYear(this DateTime date)
{
    return date.AddYears(-1);
}
public static DateTime GetCheckDate(IEnumerable<int> months, DateTime date)
{
    if (month == null || !months.Any() || months.Contains(date.Month))
        return date.FirstDayOfMonth();

    if (months.Max() < date.Month)
        return date.FirstDayOfMonth(months.Max());

    if (date.Month < months.Min())
        return date.PreviousYear().FirstDayOfMonth(months.Max());

    return date.FirstDayOfMonth(months.Where(m => m < date.Month).Max());
}

Context

StackExchange Code Review Q#44338, answer score: 10

Revisions (0)

No revisions yet.