Calculating discount rates for Present Value conversion

Question

In my team at work we have traditionally used VB, starting with 6 and moving to .NET. However, other teams in the company use C#. So, armed only with Programming C# 4.0 and VS 2013, I thought I'd have a go at it.

A relatively simple piece of functionality that I tend to use as a starter project to get used to a new language (see also my Python version) is the calculation of "discount rates", used for converting future costs to Present Values (for more information, see the Green Book, published by the UK Treasury).

This is a self-contained part of a larger project, that I have refactored for ease of transmission. In the larger project, it actually looks more like:

/Solution
    /Project
        /Generic
            ExtendedDictionary.cs
            IndexSeries.cs
            ...
        Discount.cs
        ...
    /TestProject
        UnitTestDiscount.cs
        ...
    ...

However, for the purposes of reviewing this part, I have simplified to two files, Classes.cs:

using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Serialization; 

namespace CodeReview.Generic
{
    /// <summary>
    /// Dictionary that fills in missing values.
    /// </summary>
    [Serializable]
    public class ExtendedDictionary<TValue> 
        : Dictionary<int, TValue>
    {
        /// <summary>
        /// Whether the object has yet been populated.
        /// </summary>
        private bool _Populated;

        protected ExtendedDictionary(
            SerializationInfo info,
            StreamingContext context)
            : base(info, context)
        {
            this._Populated = info.GetBoolean("_Populated");
        }

        public ExtendedDictionary() : base() 
        {
            this._Populated = false; 
        }

        public override void GetObjectData(
            SerializationInfo info, 
            StreamingContext context)
        {
            base.GetObjectData(info, context);
            info.AddValue("_Populate", this._Populate);
        }

        /// <summary>
        /// Get the value for a given key.
        /// </summary>
        /// <param name="key">The key to get.</param>
        /// <returns>The filled-in value.</returns>
        new public TValue this[int key]
        {
            get
            {
                if (!(this._Populated))
                {
                    this.Populate();
                }
                if (!this.ContainsKey(key))
                {
                    if (key > this.Keys.Max())
                    {
                        return base[this.Keys.Max()];
                    } 
                    else if (key < this.Keys.Min()) 
                    {
                        return base[this.Keys.Min()];
                    }
                }
                return base[key];
            }
        }

        /// <summary>
        /// Fill in gaps between smallest and largest keys.
        /// </summary>
        private void Populate()
        {
            int startYear = this.Keys.Min();
            var value = base[startYear];
            for (int year = startYear; year <= this.Keys.Max(); year++)
            {
                if (base.ContainsKey(year))
                {
                    value = base[year];
                }
                else
                {
                    base[year] = value;
                }
            }
            this._Populated = true;
        }
    }

    /// <summary>
    /// Indexing series of rates and factors.
    /// </summary>
    public abstract class IndexSeries
    {
        /// <summary>
        /// The zeroth year (for conversion between relative and absolute).
        /// </summary>
        protected int YearZero { get { return this._YearZero; } }
        private int _YearZero;

        /// <summary>
        /// The rates to use for calculation of the factors.
        /// </summary>
        protected ExtendedDictionary<double> Rates { get { return this._Rates; } }
        private ExtendedDictionary<double> _Rates;

        /// <summary>
        /// The factors to use for indexation.
        /// </summary>
        protected Dictionary<int, double> Factors
        {
            get { return this._Factors; }
        }
        private Dictionary<int, double> _Factors;

        /// <summary>
        /// Create a new IndexSeries.
        /// </summary>
        /// <param name="baseYear">The base (starting) year.</param>
        /// <param name="rates">The dictionary of index rates.</param>
        /// <param name="initialValue">The factor in the base year.</param>
        /// <param name="yearZero">The zeroth year (for conversion between
        /// relative and absolute.</param>
        protected IndexSeries(
            int baseYear, 
            ExtendedDictionary<double> rates,
            double initialValue,
            int yearZero)
        {
            this._Factors = new Dictionary<int, double>();
            this._Rates = rates;
            this.Factors[baseYear] = initialValue;
            this._YearZero = yearZero;
        }

        /// <summary>
        /// Get the indexation factor for a specified year.
        /// </summary>
        /// <param name="year">The year to get the factor for.</param>
        /// <returns>The factor for the specified year.</returns>
        public double Factor(int year) 
        { 
            if (!this.Factors.ContainsKey(year))
            { 
                this.FillFactors(year); 
            }
            return this.Factors[year];
        }

        /// <summary>
        /// Expose factor via "indexing" syntax.
        /// </summary>
        /// <param name="year">The year to get the factor for.</param>
        /// <returns>The factor for the specified year.</returns>
        public double this[int year] { get { return this.Factor(year); } }

        /// <summary>
        /// Define logic for filling in factors.
        /// </summary>
        /// <param name="year">The year to fill to/from.</param>
        protected abstract void FillFactors(int year);
    }
}

namespace CodeReview
{
    /// <summary>
    /// Factors for conversion between in-year prices and PV.
    /// </summary>
    public class Discount : Generic.IndexSeries
    {
        /// <summary>
        /// Default discount rates, taken from HM Treasury Green Book (2003,
        /// revised 2011).
        /// </summary>
        private static Generic.ExtendedDictionary<double> GreenBookRates =
            new Generic.ExtendedDictionary<double>()
            {{0, 0.035}, {31, 0.03}, {76, 0.025},
            {126, 0.02}, {201, 0.015}, {301, 0.01}};

        /// <summary>
        /// Create a new Discount with separate base and zeroth year.
        /// </summary>
        /// <param name="baseYear">Base year (factor = 1.0).</param>
        /// <param name="rates">The discount rates to use.</param>
        /// <param name="yearZero">Zeroth year (start counting for look-up
        /// of rates).</param>
        public Discount(
            int baseYear, 
            Generic.ExtendedDictionary<double> rates, 
            int yearZero)
            : base(baseYear, rates, 1.0, yearZero) {}

        /// <summary>
        /// Create a new Discount with the same base and zeroth year.
        /// </summary>
        /// <param name="baseYear">Base year and zeroth year.</param>
        /// <param name="rates">The discount rates to use.</param>
        public Discount(int baseYear, Generic.ExtendedDictionary<double> rates)
            : base(baseYear, rates, 1.0, baseYear) {}

        /// <summary>
        /// Fill in missing values.
        /// </summary>
        /// <param name="year">The year to fill to/from.</param>
        protected override void FillFactors(int year)
        {
            if (year < this.Factors.Keys.Min())
            {
                for (int y = year; y < this.Factors.Keys.Min(); y++)
                {
                    this.Factors[y] = 1.0;
                }
            }
            else if (!(this.Factors.ContainsKey(year)))
            {
                for (int y = this.Factors.Keys.Max() + 1; y <= year; y++)
                {
                    this.Factors[y] = (this.Factors[y - 1] / 
                        (1 + this.Rates[y - this.YearZero]));
                }
            }
        }

        /// <summary>
        /// Create Discount with default rates.
        /// </summary>
        /// <param name="baseYear">Base year (factor = 1.0).</param>
        /// <param name="yearZero">Zeroth year (start counting for look-up
        /// of rates).</param>
        /// <returns>New Discount object.</returns>
        public static Discount GreenBook(int baseYear, int yearZero)
        {
            return new Discount(baseYear, GreenBookRates, yearZero);
        }

        /// <summary>
        /// Create Discount with default rates.
        /// </summary>
        /// <param name="baseYear">Base year and zeroth year.</param>
        /// <returns>New Discount object.</returns>
        public static Discount GreenBook(int baseYear)
        {
            return Discount.GreenBook(baseYear, baseYear);
        }
    }
}

and Tests.cs:

using CodeReview;
using CodeReview.Generic;
using Microsoft.VisualStudio.TestTools.UnitTesting;

namespace TestEconomics
{
    [TestClass]
    public class UnitTestDiscount
    {
        private const double Delta = 0.001;

        private Discount SimpleDiscount;
        private Discount ComplexDiscount;

        private static ExtendedDictionary<double> GreenBookRates =
            new ExtendedDictionary<double>()
            {{0, 0.035}, {31, 0.03}, {76, 0.025},
            {126, 0.02}, {201, 0.015}, {301, 0.01}};

        [TestInitialize]
        public void SetupTest()
        {
            SimpleDiscount = new Discount(2010, GreenBookRates);
            ComplexDiscount = new Discount(2010, GreenBookRates, 2014);
        }

        [TestMethod]
        public void TestSimpleRates()
        {
            Assert.AreEqual(1, SimpleDiscount.Factor(2010), Delta);
            Assert.AreEqual(0.7089, SimpleDiscount.Factor(2020), Delta);
            Assert.AreEqual(0.0274, SimpleDiscount.Factor(2135), Delta);
        }

        [TestMethod]
        public void TestComplexRates()
        {
            Assert.AreEqual(1.0, ComplexDiscount.Factor(2010), Delta);
            Assert.AreEqual(0.7089, ComplexDiscount.Factor(2020), Delta);
            Assert.AreEqual(0.0158, ComplexDiscount.Factor(2160), Delta);
        }

        [TestMethod]
        public void TextIndexSyntax()
        {
            Assert.AreEqual(SimpleDiscount.Factor(2015),
                SimpleDiscount[2015],
                Delta);
        }
    }

    [TestClass]
    public class UnitTestGreenBook
    {
        private const double Delta = 0.001;

        private Discount GreenBookDiscount;

        [TestInitialize]
        public void SetupTest()
        {
            GreenBookDiscount =  Discount.GreenBook(2010);
        }

        [TestMethod]
        public void TestGreenBookDiscount()
        {
            Assert.AreEqual(1, GreenBookDiscount.Factor(2010), Delta);
            Assert.AreEqual(0.7089, GreenBookDiscount.Factor(2020), Delta);
            Assert.AreEqual(0.0274, GreenBookDiscount.Factor(2135), Delta);
        }
    }
}

Any feedback you have on the approach, style or structure would be great.

Answer 1

It's pretty nit-picky, but there's no real need for an else if here.

new public TValue this[int key]
{
    get
    {
        if (!this.ContainsKey(key))
        {
            if (key > this.Keys.Max())
            {
                return base[this.Keys.Max()];
            } 
            else if (key < this.Keys.Min()) 
            {
                return base[this.Keys.Min()];
            }
        }
        return base[key];
    }
}

The first part of the if returns, so there's no real need for the else. Like I said, that's pretty nit-picky and could be considered to be a matter of preference.

I don't much care for the style here.

protected ExtendedDictionary<double> Rates { get { return this._Rates; } }
private ExtendedDictionary<double> _Rates;

Either use the underscore convention or the this keyword. Using both feels.. odd. However, that too is a style nit-pick and probably up for debate.

Or maybe it's not as debatable as I thought. Right here is a case where using both obfuscated the code a bit.

protected IndexSeries(
    int baseYear, 
    ExtendedDictionary<double> rates,
    double initialValue,
    int yearZero)
{
    this._Factors = new Dictionary<int, double>();
    this._Rates = rates;
    this.Factors[baseYear] = initialValue;
    this._YearZero = yearZero;
}

Note how hard it is to tell where you're accessing the property and where you're accessing the private fields? Picking one of the two styles would make this code more clear.

protected IndexSeries(
    int baseYear, 
    ExtendedDictionary<double> rates,
    double initialValue,
    int yearZero)
{
    _factors = new Dictionary<int, double>();
    _rates = rates;
    this.Factors[baseYear] = initialValue;
    _yearZero = yearZero;
}

See how this really calls out that the difference? This way, it sticks out in the code as something that may not have been intentional. (Which, I believe it may not have been.)

Answer 2

In addition to RubberDuck's points I would like to add, that using auto properties can help a lot.

This

protected ExtendedDictionary<double> Rates { get { return this._Rates; } }
private ExtendedDictionary<double> _Rates;

can be expressed like

protected ExtendedDictionary<double> Rates { get; private set; }

If you don't need any validation logic for the setter like in your example, you can simply use the autoimplemented properties to make your code shorter and more readable. There isn't the need for a backing variable.