Transposing an object: Reflection or duplication?

Question

I have a list of objects defined by the following class:

internal class Aggregate
{
    public string fieldName { get; set; }
    public double Count { get; set; }
    public double Min { get; set; }
    public double Max { get; set; }
}

where fieldName can be Value, LowLimit or HighLimit that I want to transform into a list of objects defined as:

public class KpiValue
{
    public double Value { get; set; }
    public double? LowLimit { get; set; }
    public double? HighLimit { get; set; }
    public AggregateType AggregateType { get; set; }
}
public enum AggregateType
{
    Count,
    Min,
    Max
}

I went into it quite naively (Edited with MCVE):

var input = new List<Aggregate> {
            new Aggregate() {fieldName = "Value",Count = 5,Min = 2,Max = 3 },
            new Aggregate() {fieldName = "HighLimit",Count = 6,Min = 5,Max = 8 },
            new Aggregate() {fieldName = "LowLimit",Count = 2,Min = 9,Max = 15 } };

            List<KpiValue> values = new List<KpiValue>();
            values.Add(new KpiValue()
            {
                AggregateType = AggregateType.Count,
                Value = input.SingleOrDefault(l => l.fieldName == "Value").Count,
                HighLimit = input.SingleOrDefault(l => l.fieldName == "HighLimit").Count,
                LowLimit = input.SingleOrDefault(l => l.fieldName == "LowLimit").Count
            });
            values.Add(new KpiValue()
            {
                AggregateType = AggregateType.Min,
                Value = input.SingleOrDefault(l => l.fieldName == "Value").Min,
                HighLimit = input.SingleOrDefault(l => l.fieldName == "HighLimit").Min,
                LowLimit = input.SingleOrDefault(l => l.fieldName == "LowLimit").Min
            });
            values.Add(new KpiValue()
            {
                AggregateType = AggregateType.Count,
                Value = input.SingleOrDefault(l => l.fieldName == "Value").Max,
                HighLimit = input.SingleOrDefault(l => l.fieldName == "HighLimit").Max,
                LowLimit = input.SingleOrDefault(l => l.fieldName == "LowLimit").Max
            });

I have been looking around for a way to improve, and it seems like I could, by putting the code in a loop using Reflection as in this question

However, when reading up on how to implement it, I have also learned that reflection is not very efficient, and that it is mostly used with external libraries

In such a case, is Reflection the way to go, or should I keep my duplicated code? (Or is there a better third way?)

Edit: In this very example, performance is not crucial, as the DB calls are very long, any overhead due to reflection will remain unnoticed. I am merely trying to the the right thing™

Note: You could post this question on the [codereview.stackexchange](https://codereview.stackexchange.com/) site since you are looking for ways to improve your code. — Søren D. Ptæus, Jul 30 '18 at 13:05
Might just be me, but im not 100% sure what your code is trying to do. What is `KPIAggregateQuery`? Can you show some example input and expect output — Jamiec, Jul 30 '18 at 13:06
If the performance of reflection is crucial depends on your application. Often it´s not the few nano-seconds that affect your overall performance, but sub-optimal database-queries or obsolete recursions of the same code. So don´t over-estimate the potential affect and check if your approach *really* causes your code to be slow by *measuring*. If it is not, it´s not worth the afford to think about it too much. — MakePeaceGreatAgain, Jul 30 '18 at 13:08
@Maxime naming of it is irrelevant - I have no idea what it does, nor what you're asking for. Try rewording your question as I suggested with an example input and the expected output. How would a `Count` of a single item be of any interest - surely it would always be `1`? — Jamiec, Jul 30 '18 at 13:13
As an aside `SingleOrDefault(l => l.fieldName == "HighLimit").Count` is an error waiting to happen. `SingleOrDefault` can return `null` in which case your `.Count` will throw an immediate NullReferenceException. Either just use `Single` if you are sure it will appear (and only once) or add some null checking. — Chris, Jul 30 '18 at 13:28
Is this the whole story? If you expect multiple entries with eg `fieldName=Value` and you want to get the min/max/count of all of them, the solution looks very different. If you really only have one of each, then you've already written all the code you need. — Jamiec, Jul 30 '18 at 13:31
@Jamiec The real-world case has six more fields (average, sum, etc.) but basically, it's the whole story. What bothers me is that there is a lot of duplicated code, and it does not feel right. For example, per Chris's example, I will change SingleOrDefault to Single. I have to change it 9 times here, and 27 times in the actual code. — Maxime, Jul 30 '18 at 13:34
The problem is in data structure itself! `fieldName` could be 3+ values, but they are mapped to 3+ fields, and each Aggregate row is split to 3 rows. yes? but what is `AggregateType` from? hard codes, I only saw this... — Dongdong, Jul 30 '18 at 13:44

score 1 · Accepted Answer · answered Jul 30 '18 at 13:45

You don't really have to repeat your code, you just need to wrap your logic up in a method

public KpiValue BuildKpiValue(AggregationType aggregation, 
                              IEnumerable<Aggregate> list, 
                              Func<Aggregate, double> readValue)
{
    return new KpiValue()
    {
        AggregateType = aggregation,
        Value = readValue(list.Single(l => l.fieldName == "Value")),
        HighLimit = readValue(list.Single(l => l.fieldName == "HighLimit")),
        LowLimit = readValue(list.Single(l => l.fieldName == "LowLimit"))
    }
}

Then its just a matter of calling the method for each type of aggregation:

var input = new List<Aggregate> {
            new Aggregate() {fieldName = "Value",Count = 5,Min = 2,Max = 3 },
            new Aggregate() {fieldName = "HighLimit",Count = 6,Min = 5,Max = 8 },
            new Aggregate() {fieldName = "LowLimit",Count = 2,Min = 9,Max = 15 } };

List<KpiValue> values = new List<KpiValue>();
values.Add(BuildKpiValue(AggregationType.Count, input, agg => agg.Count));
values.Add(BuildKpiValue(AggregationType.Min, input, agg => agg.Min));
values.Add(BuildKpiValue(AggregationType.Max, input, agg => agg.Max));
// And so on for the other ones

Transposing an object: Reflection or duplication?

1 Answers1