1

I recognize this may not need to be refactored. I am simply beginning to try to employ refactoring and I failed at my attempt to do so with this line of code. I attempted to use extract method in VS2013. Alas. I use the right hand side of this line in eleven other instances.

Balance = Enumerable.Repeat(0.0, MonthsToRun).ToList();

Balance is a double, MonthsToRun is an int.

Per request:

for (int i = 0, chunksCnt = Chunks.Count; i < chunksCnt; i++)
    {
    if (Chunks[i].BeginDate < BeginDate) BeginDate = Chunks[i].BeginDate;
    MonthsToRun = Math.Max(MonthsToRun, Utils.MonthDifference(BeginDate, Chunks[i].BeginDate) + Chunks[i].Balance.Count);
    }

    Balance = Enumerable.Repeat(0.0, MonthsToRun).ToList();
    Default = Enumerable.Repeat(0.0, MonthsToRun).ToList();
    Loss = Enumerable.Repeat(0.0, MonthsToRun).ToList();
    Prepay = Enumerable.Repeat(0.0, MonthsToRun).ToList();
    Principal = Enumerable.Repeat(0.0, MonthsToRun).ToList();
    Interest = Enumerable.Repeat(0.0, MonthsToRun).ToList();

    for (int i = 0, chunksCnt = Chunks.Count; i < chunksCnt; i++)
        {
        offset = Utils.MonthDifference(BeginDate, Chunks[i].BeginDate);

        for (int j = offset, balanceCnt = Chunks[i].Balance.Count; j < (balanceCnt + offset); j++)
            {
            Balance[j] += Chunks[i].Balance[j - offset];
            Default[j] += Chunks[i].Default[j - offset];
            Loss[j] += Chunks[i].Loss[j - offset];
            Prepay[j] += Chunks[i].Prepay[j - offset];
            Principal[j] += Chunks[i].Principal[j - offset];
            Interest[j] += Chunks[i].Interest[j - offset];
            }

            if (Settings.runBacktesting)
                {
                foreach (KeyValuePair<Tuple<int, int, DateTime>, double> item in Chunks[i].TransProbChunk)
                {
                Utils.upsertDict(TransProbAgg, item.Key, item.Value);
                //Create From Status - Month Totals Dictionary to create transition rates
                Tuple<int, DateTime> key = new Tuple<int, DateTime>(item.Key.Item1, item.Key.Item3);
                Utils.upsertDict(fromMonthTotals, key, item.Value);
                }
           }
      }
StephenWinburn
  • 107
  • 1
  • 9
  • 2
    Why do you think that this needs refactoring? _"Balance is a double"_ No, then this would not compile. Maybe a `List`. – Tim Schmelter Jul 24 '14 at 13:15
  • 1
    You usually don't refactor at the line level, but rather conceptual (class, interface, etc.) level. If, however, you see a pattern that you use a lot all over your code, try to describe it. – vgru Jul 24 '14 at 13:16
  • This is more of getting my feet wet. It may not. I'm just curious, assuming it did need refactoring, what would be an appropriate manner to do so. – StephenWinburn Jul 24 '14 at 13:17
  • @TimSchmelter Thank you. Your comment on it compiling made it work. The jury is still out as to whether it is necessary, but it works. Edit: The method will now compile for the line in question, but I failed to generalize it to take arguments. The chain of fail (on my part) is now unbroken. Suggestions? – StephenWinburn Jul 24 '14 at 13:22
  • @StephenWinburn: most likely, you don't want to preallocate a list of zeros. Where is a method in your code anyway? Try to state what you're trying to do instead, this will probably get closed. – vgru Jul 24 '14 at 13:28
  • @Groo The list are initialized with zeroes and then are populated with incremental cash flows which are summed over time across different accounts in each location. So we may have $5 in time t=1 from account A and $7 from account B and so on. It has to accumulate the cah flows acorss accounts and time by looking at each account one at a time and simulating cash flows for a fixed length of time. Then it moves to the next account and determines cash flows over time which are aggregated with the cash flows from account A. Ultimately it sums over over a million accounts for each period. – StephenWinburn Jul 24 '14 at 13:32
  • @Stephen: post the entire algorithm (at least the loops doing the "updating"), that will surely have a potential for refactoring. – vgru Jul 24 '14 at 14:03

2 Answers2

1

I personally don't feel the need to refactor that line. Refactoring is usually because of readability/understanding or performance. That line seems readable as is.

If you are trying to refactor because of repetition, then would you refactor the following because '= 0;' is repeated?

int x1 = 0;
int x2 = 0;
int x3 = 0;
int x4 = 0;
int x5 = 0;
int x6 = 0;
int x7 = 0;

In this dumb case you could put it all in one line, but in real code it would become unreadable.

What you could do is make a method that "zeroes" the variables:

    private void Zero<T>(int size, ref List<T> l1, ref List<T> l2, ref List<T> l3)
    {
        T[] zeroes = Enumerable.Repeat(default(T), size).ToArray();
        l1 = new List<T>(zeroes);
        l2 = new List<T>(zeroes);
        l3 = new List<T>(zeroes);
    }

And call it like:

Zero(MonthsToRun, ref Balance, ref Default, ref Loss, ...);

In case you are trying to do it because of performance, then some kind of caching could help you...

public static class Sizer<T>
{
    static Dictionary<int, T[]> _cache = new Dictionary<int, T[]>();

    public static void Init(ref List<T> list, int size)
    {
        T[] ret;

        if (!_cache.TryGetValue(size, out ret))
        {
            ret = Enumerable.Repeat(default(T), size).ToArray();
            _cache[size] = ret;
        }

        list = ret.ToList();
    }
}

Altough I don't think you can achieve much with it...

The following tells with the above class (I did not care much about optimizing) the speedup is around 6x:

        Random r;
        const int repeatCount = 1000000;
        List<int> list = null;

        r = new Random(0);
        var start = DateTime.Now.Ticks;
        for (int i = 0; i < repeatCount; i++)
        {
            list = Enumerable.Repeat(0, r.Next(5,150)).ToList();
        }
        var end = DateTime.Now.Ticks;
        var t1 = end - start;

        r = new Random(0);
        start = DateTime.Now.Ticks;
        for (int i = 0; i < repeatCount; i++)
        {
            Sizer<int>.Init(ref list, r.Next(5, 150)); // fill the list with default values for the type
        }
        end = DateTime.Now.Ticks;
        var t2 = end - start;
        var speedup = (double)t1 / t2;
Marino Šimić
  • 7,318
  • 1
  • 31
  • 61
0

As others mentioned the result of your operation is a list of doubles. If you have eleven lines where you use Enumerable.Repeat with at least one parameter that differs every line then it would make sense to write a function (maybe an inline one) but I would let it as is because is simple enough and easy to understand.

If you need let's say a list of n zeroes in eleven places then create an array and use it where you need it.

var zeroes = Enumberable.Repeat(0.0 ,MonthsToRun).ToArray();
//or even:
//var zeroes = new double[MonthsToRun];
...
var myList1 = new List<double>(zeroes);
...
var myList2 = new List<double>(zeroes);

Inline function to use as shortcut:

Func<double, int, List<double>> rv = (val, count) => { return Enumerable.Repeat(val, count).ToList(); };
...
var myList1 = rv(0.0, MonthsToRun);
...

Also if you work with money then work with decimal:

decimal vs double! - Which one should I use and when?

Community
  • 1
  • 1
B0Andrew
  • 1,725
  • 13
  • 19