每年都是一个循环。讨厌的，嵌套的

Question

很久以前，我写了一个生成财政日历日期记录的VBA宏。这个宏一直在做这个工作，但是我需要能够在一夜之间从ETL生成财政年度，所以我在一个小型C#控制台应用程序中重写了它。

我使用了完整的依赖注入和IoC，为每个组件编写了几十个单元测试，所有的东西都很有魅力。

下面是IGeneratorService接口及其实现；这个对象是应用程序的核心，它实现日历逻辑。

我真的不喜欢5级嵌套循环；根据Visual 2013，该类的可维护性索引为89，而Generate(int,int)方法的可维护性索引为78。

namespace FiscalCalendarGenerator
{
    /// <summary>
    /// An object that encapsulates the logic to generate <see cref="FiscalCalendarModel"/> models.
    /// </summary>
    public interface IGeneratorService
    {
        /// <summary>
        /// Generates a <see cref="FiscalCalendarModel"/> for each date in the specified year.
        /// </summary>
        /// <param name="year">Fiscal year to generate records for.</param>
        /// <returns></returns>
        IEnumerable<FiscalCalendar> Generate(int year);

        /// <summary>
        /// Generates a <see cref="FiscalCalendarModel"/> for each date in the specified interval.
        /// </summary>
        /// <param name="fromYear">First fiscal year to generate records for.</param>
        /// <param name="toYear">Last fiscal year to generate records for.</param>
        IEnumerable<FiscalCalendar> Generate(int fromYear, int toYear);
    }

    public class GeneratorService : IGeneratorService
    {
        // reference start date: day 1 of Fiscal 2014 (Sunday).
        public static readonly DateTime ReferenceDate = new DateTime(2013, 12, 1);

        private readonly IFiscalYearStartDateCalculator _calculator;
        public GeneratorService(IFiscalYearStartDateCalculator calculator)
        {
            _calculator = calculator;
        }

        public IEnumerable<FiscalCalendar> Generate(int year)
        {
            return Generate(year, year);
        }

        public IEnumerable<FiscalCalendar> Generate(int fromYear, int toYear)
        {
            var currentDate = _calculator.GetFiscalYearStartDate(fromYear, ReferenceDate);
            for (var currentYear = fromYear; currentYear <= toYear; currentYear++)
            {
                var currentDayOfYear = 1;
                var currentWeekOfYear = 1;
                var currentMonthOfYear = 1;

                for (var currentQuarterOfYear = 1; currentQuarterOfYear <= 4; currentQuarterOfYear++)
                {
                    var currentDayOfQuarter = 1;
                    var currentWeekOfQuarter = 1;

                    for (var currentMonthOfQuarter = 1; currentMonthOfQuarter <= 3; currentMonthOfQuarter++)
                    {
                        var currentDayOfMonth = 1;

                        // weeks in month alternate 4-5-4 in quarter, and leap years add a 5th week in last month of year.
                        var weeksInMonth = (currentMonthOfQuarter % 2 == 0 || (currentMonthOfYear == 12 && DateTime.IsLeapYear(currentYear))) ? 5 : 4;
                        for (var currentWeekOfMonth = 1; currentWeekOfMonth <= weeksInMonth; currentWeekOfMonth++)
                        {
                            for (var currentDayOfWeek = 1; currentDayOfWeek <= 7; currentDayOfWeek++)
                            {
                                yield return new FiscalCalendar(currentDate)
                                {
                                    FiscalDayOfMonth = currentDayOfMonth,
                                    FiscalDayOfQuarter = currentDayOfQuarter,
                                    FiscalDayOfWeek = currentDayOfWeek,
                                    FiscalDayOfYear = currentDayOfYear,
                                    FiscalMonthOfQuarter = currentMonthOfQuarter,
                                    FiscalMonthOfYear = currentMonthOfYear,
                                    FiscalWeekOfMonth = currentWeekOfMonth,
                                    FiscalWeekOfQuarter = currentWeekOfQuarter,
                                    FiscalWeekOfYear = currentWeekOfYear,
                                    FiscalQuarterOfYear = currentQuarterOfYear,
                                    FiscalYear = currentYear
                                };

                                currentDate = currentDate.AddDays(1);
                                currentDayOfMonth++;
                                currentDayOfQuarter++;
                                currentDayOfYear++;
                            }

                            currentWeekOfYear++;
                        }

                        currentMonthOfYear++;
                    }
                }
            }
        }
    }
}

我想要改进的另一部分是FiscalYearStartDateCalculator类，它提供了两个方法，Visual的计算结果是它的可维护性索引为62 --我不知道该度量是如何计算的，但它与我对代码的看法几乎完全相关：

namespace FiscalCalendarGenerator
{
    /// <summary>
    /// An object responsible for calculating a fiscal year's start date.
    /// </summary>
    public interface IFiscalYearStartDateCalculator
    {
        /// <summary>
        /// Calculates the start date of specified fiscal year, given a reference start date for a reference fiscal year.
        /// </summary>
        /// <param name="fiscalYear">The fiscal year to get the start date for.</param>
        /// <param name="reference">The start date of the reference fiscal year.</param>
        /// <returns>Returns date of the first day of the specified fiscal year.</returns>
        DateTime GetFiscalYearStartDate(int fiscalYear, DateTime reference);
    }

    public class FiscalYearStartDateCalculator : IFiscalYearStartDateCalculator
    {
        public DateTime GetFiscalYearStartDate(int fiscalYear, DateTime reference)
        {
            var result = reference;
            var referenceFiscalYear = reference.Year + 1; // fiscal years start in the previous calendar year

            if (fiscalYear < referenceFiscalYear)
            {
                var years = referenceFiscalYear - fiscalYear;
                // result is 52*7 days for each year after the reference date, plus 7 days for each leap year in-between
                result = reference.AddDays(-1 * 52 * 7 * years)
                                  .AddDays(-1 * CountLeapYearsInRange(fiscalYear, reference.Year) * 7);
            }
            else if (fiscalYear > referenceFiscalYear)
            {
                var years = fiscalYear - referenceFiscalYear;
                // result is 52*7 days for each year before reference date, minus 7 days for each leap year in-between
                result = reference.AddDays(52 * 7 * years)
                                  .AddDays(CountLeapYearsInRange(fiscalYear, reference.Year) * 7);
            }

            return result;
        }

        private int CountLeapYearsInRange(int year1, int year2)
        {
            var firstYear = year1;
            var endYear = year2;

            if (year1 > year2)
            {
                firstYear = year2;
                endYear = year1;
            }
            else if (year1 == year2)
            {
                return DateTime.IsLeapYear(year1) ? 1 : 0;
            }

            return Enumerable.Range(firstYear, endYear - firstYear)
                             .Count(year => DateTime.IsLeapYear(year));
        }
    }
}

如何改进这段代码？

Answer 1

我想我应该做一个这样的私有嵌套类：

class Counter
{
    public DateTime Date            { get; set; }
    public int DayOfMonth           { get; set; }
    public int DayOfQuarter         { get; set; }
    public int DayOfWeek            { get; set; }
    public int MonthOfQuarter       { get; set; }
    [...]
}

而不是将循环分割成不同的子方法，并传递计数器对象。

public IEnumerable<FiscalCalendar> Generate(int fromYear, int toYear)
{
    var counter = new Counter();

    counter.Date = _calculator.GetFiscalYearStartDate(fromYear, ReferenceDate);

    for (var currentYear = fromYear; currentYear <= toYear; currentYear++)
    {
        foreach ( var fiscalCalender in GenerateForYear(counter, currentYear) )
            yield return fiscalCalender;
    }
}


public IEnumerable<FiscalCalendar> GenerateForYear(Counter counter, currentYear)
{
    counter.currentYear = currentYear
    counter.DayOfYear = 1;
    counter.WeekOfYear = 1;
    counter.MonthOfYear = 1;

    for (var currentQuarterOfYear = 1; currentQuarterOfYear <= 4; currentQuarterOfYear++)
    {   
        [...]
        foreach ( var fiscalCalender in GenerateQuarterOfYear(counter, currentQuarterOfYear) )
            yield return fiscalCalender;            
    }

无论如何，类似这样的事情:)

Answer 2

Bug!

我刚刚安装了ReSharper，它做了一个有趣的观察：

变量currentWeekOfQuarter = 1；

这个变量可以转换成常量-它永远不会增加.那是个虫子！

它应该与currentWeekOfYear一起递增：

currentWeekOfYear++;
currentWeekOfQuarter++;

这也意味着您至少缺少一个单元测试，否则这将很容易得到。

Answer 3

嗯，计算器--至少可以简化--因为你总是在加/减相同的内容：

 // 52*7 days for each year from the reference date, plus 7 days for each leap year in-between
 days = 52 * 7 * years + CountLeapYearsInRange(fiscalYear, reference.Year) * 7;
 if (fiscalYear < referenceFiscalYear) days = days * -1;
 return reference.AddDays(days);

但我对真正的发电机一无所知.