Calculating time windows for entities

Question

I have made some code that operates on multiple entities and creates 'time windows' for them. Basically, the entities will contain states, usually in boolean or int. They will be timestamped and in order to reduce the number of rows when creating a graph for a large date range, I am calculating time windows of 5 mins, 15 mins and 60 mins. If I'm calculating the 5 minute time windows, and a state is true from 00:00:00 to 00:02:59 and false from 00:03:00 onwards, the time window for the first 5 minutes of the day would be true, since the state was true for 3 out of 5 minutes.

I have used custom attributes, generics and reflection in order to check what properties the entities have and which of them I need to process.

The code works, but I'm sure it can be improved.

Sometimes, the code works fast for a couple of thousands of rows across 16 entities, i.e. 16 calls to the CalculateTimeWindows<T>() method (less than a second) but sometimes it's really slow (takes some 45 seconds). Any ideas how I can optimise this code?

using System;
using System.Collections.Generic;
using System.ComponentModel;
using System.Linq;
using System.Reflection;

namespace Services.Helpers
{
    #region Custom Attributes
    [AttributeUsage(AttributeTargets.Property)]
    public class DoNotCopyIntoTimeWindow : System.Attribute { } // leave default

    [AttributeUsage(AttributeTargets.Property)]
    public class IsProcessedIntoTimeWindow : System.Attribute { } // calculate time window for this property

    [AttributeUsage(AttributeTargets.Property)]
    public class IsTimeWindowDate : Attribute { } // attribute to mark property as the datetime

    [AttributeUsage(AttributeTargets.Property)]
    public class IsTimeWindowIdentifier : Attribute { } // this is the time window property
    #endregion

    public class TimeWindow
    {
        #region Structs
        public struct TimeWindowDictionary
        {
            public string Name { get; set; }
            public Dictionary<NullObject<dynamic>, int> Dictionary { get; set; }
        }

        public struct NullObject<T>
        {
            [DefaultValue(true)]
            private readonly bool isnull;// default property initializers are not supported for structs

            private NullObject(T item, bool isnull) : this()
            {
                this.isnull = isnull;
                Item = item;
            }

            public NullObject(T item) : this(item, item == null)
            {
            }

            public static NullObject<T> Null()
            {
                return new NullObject<T>();
            }

            public T Item { get; private set; }

            public bool IsNull()
            {
                return isnull;
            }

            public static implicit operator T(NullObject<T> nullObject)
            {
                return nullObject.Item;
            }

            public static implicit operator NullObject<T>(T item)
            {
                return new NullObject<T>(item);
            }

            public override string ToString()
            {
                return (Item != null) ? Item.ToString() : "NULL";
            }

            public override bool Equals(object obj)
            {
                if (obj == null)
                    return IsNull();

                if (!(obj is NullObject<T>))
                    return false;

                var no = (NullObject<T>)obj;

                if (IsNull())
                    return no.IsNull();

                if (no.IsNull())
                    return false;

                return Item.Equals(no.Item);
            }

            public override int GetHashCode()
            {
                if (IsNull())
                    return 0;

                var result = Item.GetHashCode();

                if (result >= 0)
                    result++;

                return result;
            }
        }
        #endregion

        public static IEnumerable<T> CalculateTimeWindows<T>(DateTime dateFrom, DateTime dateTo, List<T> stateModels) where T : new()
        {
            if (stateModels.Count() == 0)
                return new List<T>();

            dateFrom = GetPropertiesAndDictionaries(dateFrom, stateModels, out PropertyInfo datePropertyInfo, out List<PropertyInfo> copyProperties, out PropertyInfo timeWindowIdentifier, out int size, out TimeWindowDictionary[] dictionaries, out int i);

            return CalculateTimeWindow(dateFrom, dateTo, stateModels, 5, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i)
                        .Concat(CalculateTimeWindow(dateFrom, dateTo, stateModels, 15, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i))
                        .Concat(CalculateTimeWindow(dateFrom, dateTo, stateModels, 60, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i));
        }

        public static IEnumerable<T> CalculateTimeWindow<T>(DateTime dateFrom, DateTime dateTo, List<T> stateModels, byte timeWindowMinutes, PropertyInfo datePropertyInfo, List<PropertyInfo> copyProperties, PropertyInfo timeWindowIdentifier, int size, TimeWindowDictionary[] dictionaries, int i) where T : new()
        {
            if (stateModels.Count() > 0)
            {
                DateTime currentWindowFrom, currentWindowTo, nextWindowFrom;
                nextWindowFrom = dateFrom;
                int itemPointer = 0;
                T prevItem = default;
                T prevTimeWindow = default;

                KeyValuePair<NullObject<dynamic>, int> maxValue = new KeyValuePair<NullObject<dynamic>, int>();
                int j = 1;

                do // one time window
                {
                    for (i = 0; i < size; i++)
                        dictionaries[i].Dictionary = new Dictionary<NullObject<dynamic>, int>();

                    currentWindowFrom = nextWindowFrom;
                    nextWindowFrom = currentWindowFrom.AddMinutes(timeWindowMinutes);
                    currentWindowTo = nextWindowFrom.AddSeconds(-1);

                    var calculateTime = currentWindowFrom;

                    for (itemPointer = itemPointer; itemPointer < stateModels.Count(); itemPointer++)
                    {
                        var item = stateModels.ElementAt(itemPointer);

                        var date = (DateTime)datePropertyInfo.GetValue(item);

                        if (date >= currentWindowTo)
                            break;

                        var endDate = (date > currentWindowTo) ? nextWindowFrom : date; // state might extend more than the end of the time window
                        CalculateStateSeconds(prevItem, dictionaries, calculateTime, endDate);

                        prevItem = item;
                        calculateTime = (date < currentWindowFrom) ? currentWindowFrom : date; // to fix the 'yesterday' date
                    }

                    if (calculateTime < currentWindowTo)
                        CalculateStateSeconds(prevItem, dictionaries, calculateTime, nextWindowFrom);

                    if (dictionaries[0].Dictionary.Count > 0)
                    {
                        bool sameAsPrevious = (prevTimeWindow != null);
                        var output = new T();

                        foreach (var dictionary in dictionaries)
                        {
                            //var valToSet = dictionary.Dictionary.FirstOrDefault(x => x.Value.Equals(dictionary.Dictionary.Values.Max())).Key.Item;

                            for (i = 0; i < dictionary.Dictionary.Count; i++)
                            {
                                maxValue = dictionary.Dictionary.First();
                                for (j = 1; j < dictionary.Dictionary.Count; j++)
                                {
                                    var valuePair = dictionary.Dictionary.ElementAt(j);

                                    if (valuePair.Value > maxValue.Value)
                                        maxValue = valuePair;
                                }
                            }
                            var valToSet = maxValue.Key.Item;

                            if (sameAsPrevious)
                            {
                                var prevVal = GetValue(prevTimeWindow, dictionary.Name);

                                if (valToSet == null && prevVal == null)
                                { }
                                else if ((valToSet == null && prevVal != null) || (valToSet != null && prevVal == null) || !valToSet.Equals(prevVal))
                                    sameAsPrevious = false;
                            }

                            SetValue(output, dictionary.Name, valToSet);
                        }

                        if (!sameAsPrevious)
                        {
                            foreach (var copyProperty in copyProperties)
                                SetValue(output, copyProperty.Name, copyProperty.GetValue(prevItem));

                            timeWindowIdentifier.SetValue(output, timeWindowMinutes);

                            datePropertyInfo.SetValue(output, currentWindowFrom);

                            prevTimeWindow = output;

                            yield return output;
                        }
                    }
                }
                while (nextWindowFrom <= dateTo);
            }
        }

        private static DateTime GetPropertiesAndDictionaries<T>(DateTime dateFrom, List<T> stateModels, out PropertyInfo datePropertyInfo, out List<PropertyInfo> copyProperties, out PropertyInfo timeWindowIdentifier, out int size, out TimeWindowDictionary[] dictionaries, out int i) where T : new()
        {
            Type tType = typeof(T);
            var propInfos = tType.GetProperties();

            datePropertyInfo = propInfos.Single(p => p.GetCustomAttributes(typeof(IsTimeWindowDate), true).Any());
            var firstDate = (DateTime)datePropertyInfo.GetValue(stateModels.First());

            if (firstDate < dateFrom)
                dateFrom = new DateTime(firstDate.Year, firstDate.Month, firstDate.Day, firstDate.Hour, 0, 0, DateTimeKind.Utc);

            var properties = propInfos.Where(p => p.GetCustomAttributes(typeof(IsProcessedIntoTimeWindow), true).Any()).Select(x => x.Name);
            copyProperties = propInfos.Where(p => !p.GetCustomAttributes(typeof(IsTimeWindowIdentifier), true).Any() && !p.GetCustomAttributes(typeof(DoNotCopyIntoTimeWindow), true).Any() && !p.GetCustomAttributes(typeof(IsTimeWindowDate), true).Any() && !p.GetCustomAttributes(typeof(IsProcessedIntoTimeWindow), true).Any() && p.CanWrite && !p.GetMethod.IsVirtual).ToList();
            timeWindowIdentifier = propInfos.Single(p => p.GetCustomAttributes(typeof(IsTimeWindowIdentifier), true).Any());

            size = properties.Count();
            dictionaries = new TimeWindowDictionary[size];

            i = 0;
            foreach (var property in properties)
            {
                dictionaries[i] = new TimeWindowDictionary() { Name = property };
                i++;
            }

            return dateFrom;
        }

        private static dynamic GetValue(object inputObject, string propertyName)
        {
            Type type = inputObject.GetType();
            PropertyInfo propertyInfo = type.GetProperty(propertyName);
            return propertyInfo.GetValue(inputObject);
        }

        private static void SetValue(object inputObject, string propertyName, object propertyVal)
        {
            //find out the type
            Type type = inputObject.GetType();

            //get the property information based on the type
            PropertyInfo propertyInfo = type.GetProperty(propertyName);

            if (propertyVal != null)
            {
                //find the property type
                Type propertyType = propertyInfo.PropertyType;

                //Convert.ChangeType does not handle conversion to nullable types
                //if the property type is nullable, we need to get the underlying type of the property
                var targetType = IsNullableType(propertyType) ? Nullable.GetUnderlyingType(propertyType) : propertyType;

                //Returns an System.Object with the specified System.Type and whose value is
                //equivalent to the specified object.
                propertyVal = Convert.ChangeType(propertyVal, targetType);
            }

            //Set the value of the property
            propertyInfo.SetValue(inputObject, propertyVal, null);

        }

        private static bool IsNullableType(Type type)
        {
            return type.IsGenericType && type.GetGenericTypeDefinition().Equals(typeof(Nullable<>));
        }

        private static void CalculateStateSeconds<T>(T prevItem, IEnumerable<TimeWindowDictionary> dictionaries, DateTime calculateTime, DateTime endDate)
        {
            if (prevItem != null)
            {
                var seconds = Convert.ToInt32(endDate.Subtract(calculateTime).TotalSeconds);

                Type tType = typeof(T);

                foreach (var dictionary in dictionaries)
                {
                    PropertyInfo propertyInfo = tType.GetProperty(dictionary.Name);
                    var key = propertyInfo.GetValue(prevItem);

                    dictionary.Dictionary.TryGetValue(key, out int existingSeconds);
                    dictionary.Dictionary[key] = existingSeconds + seconds;
                }
            }
        }
    }
}

Here's a sample class and Unit test:

public abstract class MyBaseModel : ICloneable
{
    [DoNotCopyIntoTimeWindow]
    public int Id { get; set; }

    public short fk_TenantId { get; set; }

    [IsTimeWindowIdentifier]
    public byte TimeWindow { get; set; }

    [IsTimeWindowDate]
    public DateTime Date { get; set; }

    [IsProcessedIntoTimeWindow]
    public byte ActuallyAnEnum { get; set; }

    public abstract bool CalculatorOn { get; }

    public object Clone()
    {
        return this.MemberwiseClone();
    }
}

public class MyModel : MyBaseModel
{
    public short ShortId { get; set; }
    public short AnotherShortId { get; set; }

    [IsProcessedIntoTimeWindow]
    public decimal Value { get; set; }

    public override bool CalculatorOn
    {
        get
        {
            throw new NotImplementedException();
        }
    }
}

[TestMethod]
public void TestLowerResolution()
{
    /* generate test data */
    DateTime dateFrom = new DateTime(2018, 1, 28, 15, 0, 0, DateTimeKind.Utc);
    var dateTo = dateFrom.AddDays(1).AddSeconds(-1);

    var myValues = new List<MyModel>();

    myValues.Add(new MyModel()
    {
        Date = new DateTime(2018, 1, 9, 15, 48, 46, DateTimeKind.Utc),
        ShortId = 5,
        AnotherShortId = 0,
        TimeWindow = 0,
        Value = 30,
        fk_TenantId = 1006
    });

    myValues.Add(new MyModel()
    {
        Date = new DateTime(2018, 1, 29, 10, 11, 31, DateTimeKind.Utc),
        ShortId = 5,
        AnotherShortId = 0,
        TimeWindow = 0,
        Value = 14336,
        fk_TenantId = 1006
    });

    myValues.Add(new MyModel()
    {
        Date = new DateTime(2018, 1, 29, 10, 11, 59, DateTimeKind.Utc),
        ShortId = 5,
        AnotherShortId = 0,
        TimeWindow = 0,
        Value = 30,
        fk_TenantId = 1006
    });

    var windows = TimeWindow.CalculateTimeWindows(dateFrom, dateTo, myValues).ToList();
}

EDIT: Updated code at Performance issue with empty lists

Answer 1

    var firstDate = (DateTime)datePropertyInfo.GetValue(stateModels.First());

    if (firstDate < dateFrom)
      dateFrom = new DateTime(firstDate.Year, firstDate.Month, firstDate.Day, firstDate.Hour, 0, 0, DateTimeKind.Utc);

You take the first stateModel, but what if they are not ordered by date?

---

for (itemPointer = itemPointer; itemPointer < stateModels.Count(); itemPointer++)

itemPointer = itemPointer isn't neccesary:

for (; itemPointer < stateModels.Count(); itemPointer++)

---

The last parameter to CalculateTimeWindow<T>(..., ..., int i) is initialized with the out int i parameter from GetPropertiesAndDictionaries(), but the values is actually not used and i is used as an iterator index set to 0 the first time used, so why have it as a parameter and not just a local variable? Get rid of that, if it's not used.

---

This:

    public static IEnumerable<T> CalculateTimeWindows<T>(DateTime dateFrom, DateTime dateTo, List<T> stateModels) where T : new()
    {
        if (stateModels.Count() == 0)
            return new List<T>();

        dateFrom = GetPropertiesAndDictionaries(dateFrom, stateModels, out PropertyInfo datePropertyInfo, out List<PropertyInfo> copyProperties, out PropertyInfo timeWindowIdentifier, out int size, out TimeWindowDictionary[] dictionaries, out int i);

        return CalculateTimeWindow(dateFrom, dateTo, stateModels, 5, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i)
                    .Concat(CalculateTimeWindow(dateFrom, dateTo, stateModels, 15, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i))
                    .Concat(CalculateTimeWindow(dateFrom, dateTo, stateModels, 60, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i));
    }

I would write as:

  public static IEnumerable<T> CalculateTimeWindows<T>(DateTime dateFrom, DateTime dateTo, List<T> stateModels) where T : new()
  {
    if (stateModels == null || stateModels.Count() == 0)
      yield break; // return new List<T>();

    dateFrom = GetPropertiesAndDictionaries(
      dateFrom, 
      stateModels, 
      out PropertyInfo datePropertyInfo, 
      out List<PropertyInfo> copyProperties, 
      out PropertyInfo timeWindowIdentifier, 
      out int size, 
      out TimeWindowDictionary[] dictionaries, 
      out int i);

    byte[] windowDurations = { 5, 15, 60 };

    foreach (byte duration in windowDurations)
    {
      foreach (T result in CalculateTimeWindow(dateFrom, dateTo, stateModels, duration, datePropertyInfo, copyProperties, timeWindowIdentifier, size, dictionaries, i))
      {
        yield return result;
      }
    }

or as:

  public static IEnumerable<T> CalculateTimeWindows<T>(DateTime dateFrom, DateTime dateTo, List<T> stateModels) where T : new()
  {
    if (stateModels.Count() == 0)
      return new List<T>();

    dateFrom = GetPropertiesAndDictionaries(
      dateFrom,
      stateModels,
      out PropertyInfo datePropertyInfo,
      out List<PropertyInfo> copyProperties,
      out PropertyInfo timeWindowIdentifier,
      out int size,
      out TimeWindowDictionary[] dictionaries,
      out int i);

    byte[] windowDurations = { 5, 15, 60 };

    return windowDurations.SelectMany(wd =>
      CalculateTimeWindow(
        dateFrom,
        dateTo,
        stateModels,
        wd,
        datePropertyInfo,
        copyProperties,
        timeWindowIdentifier,
        size,
        dictionaries,
        i));
   }

It is much more readable and easy to maintain if you want to add a new time window.

---

    copyProperties = propInfos
      .Where(
        p => 
          !p.GetCustomAttributes(typeof(IsTimeWindowIdentifier), true).Any() 
          && !p.GetCustomAttributes(typeof(DoNotCopyIntoTimeWindow), true).Any() 
          && !p.GetCustomAttributes(typeof(IsTimeWindowDate), true).Any() 
          && !p.GetCustomAttributes(typeof(IsProcessedIntoTimeWindow), true).Any() 
          && p.CanWrite 
          && !p.GetMethod.IsVirtual).ToList();

In the above you determine which properties to fetch data from in a negative way by checking what they are not. I think, I would define a ValuePropertyAttribute to mark the properties to get data from with. It is easier to maintain, because you'll always be able to see from this attribute which properties values are copied from on an object.

---

When handling properties (setting and getting) there is a little too much conversion between PropertyInfos and strings (names) IMO.

You can reduce that by changing the definition of:

  public struct TimeWindowDictionary
  {
    public string Name { get; set; }
    public Dictionary<NullObject<dynamic>, int> Dictionary { get; set; }
  }

to

  public struct TimeWindowDictionary
  {
    public PropertyInfo PropertyInfo { get; set; }
    public Dictionary<NullObject<dynamic>, int> Dictionary { get; set; }
  }

and then change everything to accomodate to that - for instance:

    var properties = propInfos.Where(p => p.GetCustomAttributes(typeof(IsProcessedIntoTimeWindow), true).Any()).Select(x => x.Name);
    size = properties.Count();

    dictionaries = new TimeWindowDictionary[size];
    i = 0;

    foreach (var property in properties)
    {
      dictionaries[i] = new TimeWindowDictionary() { Name = property };
      i++;
    }

could be:

    dictionaries =
      propInfos
      .Where(p => p.GetCustomAttributes(typeof(IsProcessedIntoTimeWindow), true).Any())
      .Select(p => new TimeWindowDictionary { PropertyInfo = p })
      .ToArray();

    size = dictionaries.Length;

and GetValue(...) could be:

  private static dynamic GetValue(object inputObject, PropertyInfo propertyInfo)
  {
    return propertyInfo.GetValue(inputObject);
    //Type type = inputObject.GetType();
    //System.Reflection.PropertyInfo propertyInfo = type.GetProperty(propertyName);
    //return propertyInfo.GetValue(inputObject);
  }

and SetValue(...) could likewise have the signature:

  private static void SetValue(object inputObject, PropertyInfo propertyInfo, object propertyVal)

and in CalculateStateSeconds<T>(...):

        //PropertyInfo propertyInfo = tType.GetProperty(dictionary.Name);
        var key = dictionary.PropertyInfo.GetValue(prevItem);
        dictionary.Dictionary.TryGetValue(key, out int existingSeconds);
        dictionary.Dictionary[key] = existingSeconds + seconds;

---

  private static DateTime GetPropertiesAndDictionaries<T>(
    DateTime dateFrom,
    List<T> stateModels,
    out PropertyInfo datePropertyInfo,
    out List<PropertyInfo> copyProperties,
    out PropertyInfo timeWindowIdentifier,
    out int size,
    out TimeWindowDictionary[] dictionaries,
    out int i) where T : new()
  {

When having all these arguments passed from method to method, I think I would make a container object for them and pass that instead. Alternatively you could make the class statefull with these parameters as members instead, so you can avoid having arguments to the methods at all. The methods should then be non static members of course. Your one and only static method could then be something like:

  public static IEnumerable<T> GetTimeWindows<T>(DateTime dateFrom, DateTime dateTo, List<T> stateModels) where T : new()
  {
    TimeWindow timeWindow = new TimeWindow(dateFrom, dateTo, stateModels);
    return timeWindow.Calculate();
  }

where the TimeWindow class should have an appropriate generric type parameter.

Answer 2

Alternative approach

Instead of using attributes and reflection, I'd go for a set of generic methods that take type-specific date and value-selector methods as parameters.

First, a method that returns items grouped by time segment:

public static IEnumerable<TimeSegmentItems<T>> GetItemsPerTimeSegment(
    IEnumerable<T> items,
    DateTime from,
    DateTime to,
    TimeSpan segmentDuration,
    Func<T, DateTime> getTimestamp) { ... }

// Contains all items within a given time segment,
// as well as the last item from the preceding time segment
// (that item determines the initial state):
public class TimeSegmentItems<T>
{
    public DateTime Start { get; }
    public DateTime End { get; }
    public IEnumerable<T> Items { get; }
}

Then, a utility method for obtaining the dominant value for a given time segment:

public static TValue GetDominantValue<TItem, TValue>(
    IEnumerable<TItem> items,
    DateTime from,
    DateTime to,
    Func<TItem, DateTime> getTimestamp,
    Func<TItem, TValue> getValue) { ... }

Together, they allow you to do the following:

GetItemsPerTimeSegment(myValues, fromDate, toDate, TimeSpan.FromMinutes(5), m => m.Date)
    .Select(segment => new MyModel {
        Value = GetDominantValue(segment.Items, segment.Start, segment.End, m => m.Date, m => m.Value),
        ActuallyAnEnum = GetDominantValue(segment.Items, segment.Start, segment.End, m => m.Date, m => m.ActuallyAnEnum),
    });

Which can further be streamlined, but that's the gist of it.

Comparison

Comparing this to your original approach:

• Reflection and dynamic are both relatively slow, and especially with dynamic you lose compile-time checking. This approach should be more succinct, more robust and (much) faster.
• With attributes, you are limited to types that you can add attributes to. This approach lets you work with any type, and even create different 'views' for the same type, at the cost of having to write type-specific logic (the date and value selectors, and the final result-building code).
• Instead of a single public method that does everything, these utility methods each focus on a specific task, which makes them more reusable. You can swap the dominant-value logic for average-value logic without having to touch the time-segment grouping logic. You can also build an attribute/reflection-based layer on top of this that removes the need for type-specific code, if necessary.

Answer 3

I found something that looks really weird to me

    for (i = 0; i < dictionary.Dictionary.Count; i++)
    {
        maxValue = dictionary.Dictionary.First();
        for (j = 1; j < dictionary.Dictionary.Count; j++)
        {
            var valuePair = dictionary.Dictionary.ElementAt(j);

            if (valuePair.Value > maxValue.Value)
                maxValue = valuePair;
        }
    }
    var valToSet = maxValue.Key.Item;

This looks like it could be replaced with

var valToSet = dictionary.Values.Max();

if you are trying to get the key with the max value you could use this

var valToSet = dictionary.OrderByDescending(x => x.Value).First().Key;

found on https://stackoverflow.com/a/42508315/1214743 be sure to test around values that are the same.

---

I also found something else that I would do differently as well

  if (sameAsPrevious)
  {
      var prevVal = GetValue(prevTimeWindow, dictionary.Name);

      if (valToSet == null && prevVal == null)
      { }
      else if ((valToSet == null && prevVal != null) || (valToSet != null && prevVal == null) || !valToSet.Equals(prevVal))
          sameAsPrevious = false;
  }

your if/else statement could be written like this:

if (valToSet == null && prevVal == null)
{ 
}
else
{
    sameAsPrevious = (valToSet == preval);
}

because

• if valToSet is null and the other has a value it won't be the same, it also works the other way around. This would make sameAsPrevious == false
• if they are both null they get caught in the initial if statement
• if they are both have a value and it is the same we want the sameAsPrevious value to be true anyway. if valToSet == preval will set sameAsPrevious to true, which it should be in that situation.