6
\$\begingroup\$

I made a method that takes 2 DateTimes as a parameter, a startDate and a stopDate.

The routine should return a list of DateTime ranges by checking if the range in the parameters is above one year TimeSpan. If it is, it should separate the range into smaller ranges of one year, starting form startDate and adding a year until reaching the limit, and then adjusting the last range accordingly (as it might only contain a few months).

Can this method be made better or even recursive?

// startDate = 2016-01-01        
// endDate = 2020-06-01

List<Tuple<DateTime,DateTime>> list = GetYearsBetweenDates(startDate, endDate);

//list = 2016-01-01 2016-12-31 
//       2017-01-01 2017-12-31 
//       2018-01-01 2018-12-31 
//       2019-01-01 2019-12-31 
//       2020-01-01 2020-06-01

private List<Tuple<DateTime, DateTime>> GetYearsBetweenDates(DateTime startDate, DateTime stopDate)
{
    var list = new List<Tuple<DateTime, DateTime>>();
    var oneYear = TimeSpan.FromDays(365);
    var tempStopDate = startDate + oneYear;
    var tempStartDate = startDate;
    bool first = true;

    if (stopDate - startDate < oneYear)
    {
        list.Add(new Tuple<DateTime, DateTime>(startDate, stopDate));
        return list;
    }

    while (tempStopDate != stopDate)
    {
        list.Add(new Tuple<DateTime, DateTime>(tempStartDate, tempStopDate));
        if (first)
        {
            tempStartDate += oneYear.Add(TimeSpan.FromDays(1)); // We should not have the same date twice
            first = false;
        }
        else
            tempStartDate += oneYear;

        tempStopDate += oneYear;

        if (tempStopDate > stopDate)
        {
            tempStopDate = stopDate;
            list.Add(new Tuple<DateTime, DateTime>(tempStartDate, stopDate));
        }
    }

    return list;
}
\$\endgroup\$
  • \$\begingroup\$ A year is not 365 days long. \$\endgroup\$ – Roland Illig Nov 7 '16 at 20:42
  • \$\begingroup\$ @RolandIllig True, but irrelevant for this question. \$\endgroup\$ – Nicholas Nov 7 '16 at 20:50
  • \$\begingroup\$ @Nicholas on the contrary, it produces an invalid end-date since the first leapyear in the range and it continues to grow every four years \$\endgroup\$ – t3chb0t Nov 8 '16 at 5:03
4
\$\begingroup\$

If you perform all calcuclations dynamically your method becomes a simple few lines long loop:

private IEnumerable<DateTimeRange> GetYearsBetweenDates(DateTime startDate, DateTime stopDate)
{
    for (int i = startDate.Year; i <= stopDate.Year; i++)
    {
        yield return new DateTimeRange
        (
            start: new DateTime(i, startDate.Month, startDate.Day),
            end:
             i == stopDate.Year 
             ? new DateTime(i, stopDate.Month, stopDate.Day).AddYears(1) 
             : new DateTime(i, startDate.Month, startDate.Day).AddYears(1)
        );
    }
}

So what I have I changed?

The method returns an IEnumerable<T> and uses the yield return which makes it deferred (this means it is executed only if enumerated).

The T is a new struct that is easier to understand than a Tuple:

struct DateTimeRange
{
    public DateTimeRange(DateTime start, DateTime end)
    {
        Start = start;
        End = end;
    }
    public DateTime Start { get; }
    public DateTime End { get; }
}

To get all dates you call it with ToList or ToArray

var dates = GetYearsBetweenDates(startDate, endDate).ToList();

or you put it in a foreach loop

foreach(var range in GetYearsBetweenDates(startDate, endDate))
{
    // do something
}

You can also exagerate and make it pure LINQ method without any manual loop but personaly I think the for loop looks better:

private IEnumerable<DateTimeRange> GetYearsBetweenDates3(DateTime startDate, DateTime stopDate)
{
    return Enumerable.Range(startDate.Year, stopDate.Year - startDate.Year + 1)
        .Select(x => new DateTimeRange
        (
            start: new DateTime(x, startDate.Month, startDate.Day),
            end:
             x == stopDate.Year 
             ? new DateTime(x, stopDate.Month, stopDate.Day).AddYears(1) 
             : new DateTime(x, startDate.Month, startDate.Day).AddYears(1)
        ));
}

What else?

var list = new List<Tuple<DateTime, DateTime>>();

You should always choose meaningful names for all variables. You did it well for the others. This could have been results or dates.

var oneYear = TimeSpan.FromDays(365);

It's safer to use the AddYear(1) method rather then hardcoding the number. Especially if you don't want to care about leapyears.

\$\endgroup\$
  • \$\begingroup\$ Upvoted, but not convinced about DateTimespan... what's wrong with TimeSpan? \$\endgroup\$ – Mathieu Guindon Nov 7 '16 at 22:48
  • 1
    \$\begingroup\$ @Mat'sMug well, the TimeSpan has no beginning, it has only a duration but here you want to have Start & End points. Perhaps the DateTimespan isn't a perfect name but I couldn't come up with something better. I guess DateTimeRange would have been more appropriate. \$\endgroup\$ – t3chb0t Nov 8 '16 at 4:44
  • \$\begingroup\$ Fair enough, that does justify it :) \$\endgroup\$ – Mathieu Guindon Nov 8 '16 at 4:45
  • \$\begingroup\$ I've renamed it to DateTimeRange. \$\endgroup\$ – t3chb0t Nov 8 '16 at 11:01
  • \$\begingroup\$ @Nicholas you can ask Mat's Mug about merging accounts if you want to, if I'm correct you're also the user122455, right? \$\endgroup\$ – t3chb0t Nov 8 '16 at 20:48

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.