5
\$\begingroup\$

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<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 = new moveDateDelegate(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 = new moveDateDelegate(MyDelegateClass.daily);
                    break;
                case 'W':
                    moveDate = new moveDateDelegate(MyDelegateClass.weekly);
                    break;
                case 'M':
                    moveDate = new moveDateDelegate(MyDelegateClass.monthly);
                    break;
                case 'Q':
                    moveDate = new moveDateDelegate(MyDelegateClass.quarterly);
                    break;
                case 'S':
                    moveDate = new moveDateDelegate(MyDelegateClass.semily);
                    break;
                case 'A':
                    moveDate = new moveDateDelegate(MyDelegateClass.annually);
                    break;
                default:
                    moveDate = new moveDateDelegate(MyDelegateClass.daily);
                    break;
            }
        }


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

I know there's still a lot of duplicated code, but I don't see how to get around this. Anonymous methods? That seems messier.

I browsed a few other questions, which sort of steer toward dictionaries of actions, which I'm not able to grok, and I don't see how it improves.

\$\endgroup\$
1
  • \$\begingroup\$ oh yeah, I know I should use LINQ to build up the list, but business reasons require .NET 2.0. \$\endgroup\$
    – Snowbody
    May 16, 2014 at 19:06

1 Answer 1

6
\$\begingroup\$

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<T>, consider IEnumerable<T> or IList<T>.

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<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':
                    moveDate = MyDelegateClass.Semily;
                    break;
                case 'A':
                    moveDate = MyDelegateClass.Annually;
                    break;
                default:
                    moveDate = MyDelegateClass.Daily;
                    break;
            }
        }


        do
        {
            retval.Add(
                new KeyValuePair<double, string[]>(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<DateTime, DateTime>. Cut #2:

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;
    }
}

Can we do better? Yes! Let's employ a IDictionary<K,V> in place of the case statements. Cut #3:

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;
    }
}

Can we do more? Possibly. Your turn :)

\$\endgroup\$
5
  • \$\begingroup\$ the compiler doesn't like date=>date.AddDays(1) ; it says "Cannot convert lambda expression to type ... because it is not a delegate type" \$\endgroup\$
    – Snowbody
    May 16, 2014 at 21:34
  • 2
    \$\begingroup\$ Ah, I see your comment now that you are using .NET 2.0. Re-introduce MoveDateDelegate and then replace all occurrences Func<DateTime, DateTime> with it. \$\endgroup\$ May 16, 2014 at 21:39
  • \$\begingroup\$ That did it! Thanks! So glad it didn't require a total rewrite. \$\endgroup\$
    – Snowbody
    May 16, 2014 at 22:10
  • \$\begingroup\$ One more question @Jesse . You say "Code to interfaces." But how does that benefit anybody if the code always returns a List<>? \$\endgroup\$
    – Snowbody
    May 21, 2014 at 13:56
  • 2
    \$\begingroup\$ Your caller shouldn't have to know the concrete implementation. If they're just iterating over a collection, return a collection (enumerable). You can then change your implementation to be a List, Array, LinkedList, etc. and the caller wouldn't have to change. \$\endgroup\$ May 21, 2014 at 14:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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