6
\$\begingroup\$

I am writing some code to transform an array of decimals so that they are discounted over time.

I initially wrote the following code:

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    decimal[] deflatedItems = new decimal[items.Count()];

    decimal factorAdjustment = 1;

    for (int i = 0; i < items.Length; i++)
    {
        deflatedItems[i] = items[i] / factorAdjustment;
        factorAdjustment = factorAdjustment * (1 + deflateRate);
    }

    return deflatedItems;
} 

Then I realised that instead of having the factorAdjustment variable, I can just use to the power of:

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    decimal[] deflatedItems = new decimal[items.Count()];

    for (int i = 0; i < items.Length; i++)
    {
        deflatedItems[i] = items[i] / (decimal)Math.Pow((double)(1 + deflateRate), i);
    }

    return deflatedItems;
}

As Math.Pow() only accepts and returns doubles, I think it's easier to create a wrapper function for it:

private decimal PowerOf(decimal x, decimal y)
{
    return (decimal)Math.Pow((double)x, (double)y);
}

and then simplify:

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    decimal[] deflatedItems = new decimal[items.Count()];

    for (int i = 0; i < items.Length; i++)
    {
        deflatedItems[i] = items[i] / PowerOf(1 + deflateRate, i);
    }

    return deflatedItems;
}

Then I realised that I really just want to transform the array and don't need a second array:

private void deflateArray(decimal[] items, decimal deflateRate)
{
    for (int i = 0; i < items.Length; i++)
    {
        items[i] = items[i] / PowerOf(1 + deflateRate, i);
    }
}

and then I can simplify this to:

private void deflateArray(decimal[] items, decimal deflateRate)
{
    for (int i = 0; i < items.Length; i++)
    {
        items[i] /= PowerOf(1 + deflateRate, i);
    }
}

so my final version is:

private void deflateArray(decimal[] items, decimal deflateRate)
{
    for (int i = 0; i < items.Length; i++)
    {
        items[i] /= PowerOf(1 + deflateRate, i);
    }
}

private decimal PowerOf(decimal x, decimal y)
{
    return (decimal)Math.Pow((double)x, (double)y);
}

I'm sure it can probably done in a one-liner by someone who is cleverer than me. Which version is the most easily understood?

I am not really worried about the performance as the array of items will only ever have 100 or 200 items and it's never done in a loop.

\$\endgroup\$
  • 1
    \$\begingroup\$ @Lachlan Barclay your final version and original do not give the same result. In the original, the first item is discounted by 1 + deflateRate, but, in the final version, the first item is discounted by PowerOf(1m + deflateRate, i) where i == 0. This gives a divisor of 1. \$\endgroup\$ – AlanT Apr 28 '17 at 8:48
  • \$\begingroup\$ Thanks @AlanT :) Someone else found that bug a few days later!!! I'll fix up the code :) \$\endgroup\$ – Rocklan Apr 30 '17 at 23:54
8
\$\begingroup\$

Arguably a better variant for DeflateArray:

private decimal[] DeflateArray(decimal[] items, decimal deflateRate)
{
    return items
        .Select((item, index) => item /= PowerOf(1 + deflateRate, index))
        .ToArray();
}

It's better in a way it helps avoiding OBOE.

And by the way, another thing you can do is define it as an extension method in a separate static class, and will be able to invoke it as myArrayOfDecimals.DeflateArrayWithRate(1.5m):

public static class DecimalArrayExtensions
{
    public static decimal[] DeflateArrayWithRate(this decimal[] items, decimal deflateRate)
    {
        return items
            .Select((item, index) => item /= PowerOf(1 + deflateRate, index))
            .ToArray();
    }

    private static decimal PowerOf(decimal x, decimal y)
    {
        return (decimal)Math.Pow((double)x, (double)y);
    }
}
\$\endgroup\$
1
\$\begingroup\$

I like your original. It is more efficient and reads clearly to me. It would be an edge edge case but you could have a factorAdjustment that is not represented perfect as double.

Don't use both Count() and Length.

private decimal[] deflateArray(decimal[] items, decimal deflateRate)
{
    int count = items.Count();
    decimal[] deflatedItems = new decimal[count];

    decimal factorAdjustment = 1 + deflateRate;

    for (int i = 0; i < count; i++)
    {
        deflatedItems[i] = items[i] / factorAdjustment;
        factorAdjustment *= (1 + deflateRate);
    }

    return deflatedItems;
} 
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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