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

Financial app for retrieving data over a set of dates

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

Problem

I'm writing a financial app that has to get data over a set of dates.

The user specifies the start date, end date, and frequency from a list of choices. (daily, weekly, monthly, quarterly, semi, annually).

There's also a preference option that allows the user to choose whether each period should be the same length ("monthly" = every 30 days vs. same day number of every month)

To avoid redoing the comparison in the loop, and to avoid duplicating code, 'use delegates' popped into my head. Suggestions please? Be brutal, it's the best way for me to learn.

```
public partial class FinancialApp
{

delegate void moveDateDelegate(ref DateTime d);

class MyDelegateClass
{
public int dateinterval;
public void quickAdd(ref DateTime d)
{
d = d.AddDays(dateinterval);
}
public static void daily(ref DateTime d)
{
d = d.AddDays(1);
}
public static void weekly(ref DateTime d)
{
d = d.AddDays(7);
}
public static void monthly(ref DateTime d)
{
d = d.AddMonths(1);
}
public static void quarterly(ref DateTime d)
{
d = d.AddMonths(3);
}
public static void semily(ref DateTime d)
{
d = d.AddMonths(6);
}
public static void annually(ref DateTime d)
{
d = d.AddYears(1);
}
}

static List> getAllCurveTickers(string security,
double startdate,
double enddate,
char freq,
bool? fill,
bool quick)
{
var retval = new List>();
DateTime d = DateTime.FromOADate(startdate);
moveDateDelegate moveDate;
MyDelegateClass mydc=new MyDelegateClass();

if (quick)
{
d = DateTime.Today;
moveDate = new moveDateDelegate(mydc.quickAdd);
switch (freq)
{
case 'D':
myd

Solution

Naming

I highly recommend you read through the C# naming conventions. Highlights are:

  • Private fields are lowerCamelCase, sometimes prepended with an underscore



  • Properties are UpperCamelCase



  • Methods are UpperCamelCase



  • Local variables are lowerCamelCase



Usage

  • Do not use public member variables (dateinterval). Make them private and front with a property.



  • ref parameters are not really needed here at all. Change the return type from void to DateTime and just return it.



  • new moveDateDelegate is not necessary since .NET 1.1. It's just code noise. The syntax is cleaner as moveDate = MyDelegateClass.daily; for example.



  • Code to interfaces. Instead of returning List, consider IEnumerable or IList.



Given these, here is a first cut at a rewrite:

public partial class FinancialApp
{

    private delegate DateTime moveDateDelegate(DateTime d);

    private class MyDelegateClass
    {
        private int dateinterval;

        public int DateInterval
        {
            get
            {
                return this.dateinterval;
            }

            set
            {
                this.dateinterval = value;
            }
        }

        public DateTime QuickAdd(DateTime d)
        {
            return d.AddDays(dateinterval);
        }

        public static DateTime Daily(DateTime d)
        {
            return d.AddDays(1);
        }

        public static DateTime Weekly(DateTime d)
        {
            return d.AddDays(7);
        }

        public static DateTime Monthly(DateTime d)
        {
            return d.AddMonths(1);
        }

        public static DateTime Quarterly(DateTime d)
        {
            return d.AddMonths(3);
        }

        public static DateTime Semily(DateTime d)
        {
            return d.AddMonths(6);
        }

        public static DateTime Annually(DateTime d)
        {
            return d.AddYears(1);
        }
    }

    private static IEnumerable> GetAllCurveTickers(
        string security,
        double startdate,
        double enddate,
        char freq,
        bool? fill,
        bool quick)
    {
        var retval = new List>();
        DateTime d = DateTime.FromOADate(startdate);
        moveDateDelegate moveDate;
        MyDelegateClass mydc = new MyDelegateClass();

        if (quick)
        {
            d = DateTime.Today;
            moveDate = mydc.QuickAdd;
            switch (freq)
            {
                case 'D':
                    mydc.DateInterval = 1;
                    break;
                case 'W':
                    mydc.DateInterval = 7;
                    break;
                case 'M':
                    mydc.DateInterval = 30;
                    break;
                case 'Q':
                    mydc.DateInterval = 90;
                    break;
                case 'S':
                    mydc.DateInterval = 182;
                    break;
                case 'A':
                    mydc.DateInterval = 365;
                    break;
                default:
                    mydc.DateInterval = 1;
                    break;
            }
        }
        else
        {
            d = DateTime.FromOADate(startdate);
            switch (freq)
            {
                case 'D':
                    moveDate = MyDelegateClass.Daily;
                    break;
                case 'W':
                    moveDate = MyDelegateClass.Weekly;
                    break;
                case 'M':
                    moveDate = MyDelegateClass.Monthly;
                    break;
                case 'Q':
                    moveDate = MyDelegateClass.Quarterly;
                    break;
                case 'S':
                    moveDate = MyDelegateClass.Semily;
                    break;
                case 'A':
                    moveDate = MyDelegateClass.Annually;
                    break;
                default:
                    moveDate = MyDelegateClass.Daily;
                    break;
            }
        }

        do
        {
            retval.Add(
                new KeyValuePair(d.ToOADate(), MakeIcurveTickers(security, d.ToOADate(), fill)));
            d = moveDate.Invoke(d);
        }
        while (startdate <= enddate);
        return retval;
    }
}


Can we do better? Yes. Lambda expressions can remove the need for your entire MoveDateDelegate and MyDelegateClass. MoveDateDelegate gets replaced with Func. Cut #2:

```
public partial class FinancialApp
{
private static IEnumerable> GetAllCurveTickers(
string security,
double startdate,
double enddate,
char freq,
bool? fill,
bool quick)
{
var retval = new List>();
DateTime d = DateTime.FromOADate(startdate);
Func moveDate;

if (quick)
{
int interval;
d = DateTime.Today;
switch (freq)
{
case 'D':

Code Snippets

public partial class FinancialApp
{

    private delegate DateTime moveDateDelegate(DateTime d);

    private class MyDelegateClass
    {
        private int dateinterval;

        public int DateInterval
        {
            get
            {
                return this.dateinterval;
            }

            set
            {
                this.dateinterval = value;
            }
        }

        public DateTime QuickAdd(DateTime d)
        {
            return d.AddDays(dateinterval);
        }

        public static DateTime Daily(DateTime d)
        {
            return d.AddDays(1);
        }

        public static DateTime Weekly(DateTime d)
        {
            return d.AddDays(7);
        }

        public static DateTime Monthly(DateTime d)
        {
            return d.AddMonths(1);
        }

        public static DateTime Quarterly(DateTime d)
        {
            return d.AddMonths(3);
        }

        public static DateTime Semily(DateTime d)
        {
            return d.AddMonths(6);
        }

        public static DateTime Annually(DateTime d)
        {
            return d.AddYears(1);
        }
    }

    private static IEnumerable<KeyValuePair<double, string[]>> GetAllCurveTickers(
        string security,
        double startdate,
        double enddate,
        char freq,
        bool? fill,
        bool quick)
    {
        var retval = new List<KeyValuePair<double, string[]>>();
        DateTime d = DateTime.FromOADate(startdate);
        moveDateDelegate moveDate;
        MyDelegateClass mydc = new MyDelegateClass();

        if (quick)
        {
            d = DateTime.Today;
            moveDate = mydc.QuickAdd;
            switch (freq)
            {
                case 'D':
                    mydc.DateInterval = 1;
                    break;
                case 'W':
                    mydc.DateInterval = 7;
                    break;
                case 'M':
                    mydc.DateInterval = 30;
                    break;
                case 'Q':
                    mydc.DateInterval = 90;
                    break;
                case 'S':
                    mydc.DateInterval = 182;
                    break;
                case 'A':
                    mydc.DateInterval = 365;
                    break;
                default:
                    mydc.DateInterval = 1;
                    break;
            }
        }
        else
        {
            d = DateTime.FromOADate(startdate);
            switch (freq)
            {
                case 'D':
                    moveDate = MyDelegateClass.Daily;
                    break;
                case 'W':
                    moveDate = MyDelegateClass.Weekly;
                    break;
                case 'M':
                    moveDate = MyDelegateClass.Monthly;
                    break;
                case 'Q':
                    moveDate = MyDelegateClass.Quarterly;
                    break;
                case 'S':
     
public partial class FinancialApp
{
    private static IEnumerable<KeyValuePair<double, string[]>> GetAllCurveTickers(
        string security,
        double startdate,
        double enddate,
        char freq,
        bool? fill,
        bool quick)
    {
        var retval = new List<KeyValuePair<double, string[]>>();
        DateTime d = DateTime.FromOADate(startdate);
        Func<DateTime, DateTime> moveDate;

        if (quick)
        {
            int interval;
            d = DateTime.Today;
            switch (freq)
            {
                case 'D':
                    interval = 1;
                    break;
                case 'W':
                    interval = 7;
                    break;
                case 'M':
                    interval = 30;
                    break;
                case 'Q':
                    interval = 90;
                    break;
                case 'S':
                    interval = 182;
                    break;
                case 'A':
                    interval = 365;
                    break;
                default:
                    interval = 1;
                    break;
            }

            moveDate = date => date.AddDays(interval);
        }
        else
        {
            d = DateTime.FromOADate(startdate);
            switch (freq)
            {
                case 'D':
                    moveDate = date => date.AddDays(1);
                    break;
                case 'W':
                    moveDate = date => date.AddDays(7);
                    break;
                case 'M':
                    moveDate = date => date.AddMonths(1);
                    break;
                case 'Q':
                    moveDate = date => date.AddMonths(3);
                    break;
                case 'S':
                    moveDate = date => date.AddMonths(6);
                    break;
                case 'A':
                    moveDate = date => date.AddYears(1);
                    break;
                default:
                    moveDate = date => date.AddDays(1);
                    break;
            }
        }

        do
        {
            retval.Add(
                new KeyValuePair<double, string[]>(d.ToOADate(), MakeIcurveTickers(security, d.ToOADate(), fill)));
            d = moveDate.Invoke(d);
        }
        while (startdate <= enddate);
        return retval;
    }
}
public partial class FinancialApp
{
    private static readonly IDictionary<char, Func<DateTime, DateTime>> methodMap =
        new Dictionary<char, Func<DateTime, DateTime>>
        {
            { 'D', date => date.AddDays(1) },
            { 'W', date => date.AddDays(7) },
            { 'M', date => date.AddMonths(1) },
            { 'Q', date => date.AddMonths(3) },
            { 'S', date => date.AddMonths(6) },
            { 'A', date => date.AddYears(1) }
        };

    private static readonly IDictionary<char, int> intervalMap =
        new Dictionary<char, int>
        {
            { 'D', 1 },
            { 'W', 7 },
            { 'M', 30 },
            { 'Q', 90 },
            { 'S', 182 },
            { 'A', 365 }
        };

    private static IEnumerable<KeyValuePair<double, string[]>> GetAllCurveTickers(
        string security,
        double startdate,
        double enddate,
        char freq,
        bool? fill,
        bool quick)
    {
        var retval = new List<KeyValuePair<double, string[]>>();
        DateTime d = DateTime.FromOADate(startdate);
        Func<DateTime, DateTime> moveDate;

        if (quick)
        {
            int interval;
            d = DateTime.Today;
            if (!intervalMap.TryGetValue(freq, out interval))
            {
                interval = 1;
            }

            moveDate = date => date.AddDays(interval);
        }
        else
        {
            d = DateTime.FromOADate(startdate);
            if (!methodMap.TryGetValue(freq, out moveDate))
            {
                moveDate = date => date.AddDays(1);
            }
        }

        do
        {
            retval.Add(
                new KeyValuePair<double, string[]>(d.ToOADate(), MakeIcurveTickers(security, d.ToOADate(), fill)));
            d = moveDate.Invoke(d);
        }
        while (startdate <= enddate);
        return retval;
    }
}

Context

StackExchange Code Review Q#50951, answer score: 6

Revisions (0)

No revisions yet.