4
\$\begingroup\$

I have a 'speed problem' with a program im currently developing. I already have encountered the methods which costs some time.

public void ConvertDataFrameToValueFields()
    {
        List<byte[]> items = new List<byte[]>() { new byte[2], new byte[2], new byte[2], new byte[2] };

        items[0][0] = this.DataBytes[0];
        items[0][1] = this.DataBytes[1];
        items[1][0] = this.DataBytes[2];
        items[1][1] = this.DataBytes[3];
        items[2][0] = this.DataBytes[4];
        items[2][1] = this.DataBytes[5];
        items[3][0] = this.DataBytes[6];
        items[3][1] = this.DataBytes[7];

        int i = 0;
        IEnumerable<PropertyInfo> result = this.determineCanMessageValueFields();

        foreach (var prop in result)
        {
            if (prop.PropertyType == typeof(MessageStatusValueField))
            {
                prop.SetValue(this, new MessageStatusValueField(BitConverter.ToUInt16(items[i], 0), prop.Name, this.UnitType), null);
            }
            else if (prop.PropertyType == typeof(MessageValueField))
            {
                prop.SetValue(this, new MessageValueField(BitConverter.ToUInt16(items[i], 0), prop.Name), null);
            }
            else if (prop.PropertyType == typeof(MessageValueFieldInt32))
            {
                prop.SetValue(this, new MessageValueFieldInt32(BitConverter.ToUInt16(items[i], 0), prop.Name), null);
            }
            else if (prop.PropertyType.GetCustomAttributes(false).Count(x => x.GetType() == typeof(BitmaskValueFieldClass)) > 0)
            {
                var tempField = new MessageValueField(BitConverter.ToUInt16(items[i], 0), prop.Name);
                prop.SetValue(this, Convert.ChangeType(tempField, prop.PropertyType), null);
            }
            i++;
        }
    }




     private IEnumerable<PropertyInfo> determineCanMessageValueFields()
            {
                //Get all properties with the 
                //custom attribute [IsValueField]
                return this.GetType().GetProperties()
                                     .Where(p => p.GetCustomAttributes(false)
                                     .Count(x => x.GetType() == typeof(IsValueField)) > 0)
                                     .OrderBy(x => ((IsValueField)x.GetCustomAttributes(false)[0]).ValueFieldIndex);
            }

 public object ToType(Type conversionType, IFormatProvider provider)
        {
            //...only convert if the conversion type is a BitmaskValueFieldClass
            if (conversionType.GetCustomAttributes(false).Count(x => x.GetType() == typeof(BitmaskValueFieldClass)) == 0)
            {
                throw new NotImplementedException("The conversion is only available for conversion types with the BitmaskValueFieldClass attribute");
            }

            //...create an instance of the conversion type
            var instance = Activator.CreateInstance(conversionType);

            //...go thru each property and set the value
            foreach (var item in instance.GetType().BaseType.GetProperties())
            {
                item.SetValue(instance, this.GetType().GetProperty(item.Name).GetValue(this, null), null);
            }

            return instance;
        }

The method ConvertDataFrameToValueFields() is the sticking point. I know it is because of the prop.SetValue() and the ToType() IConvertible method calls. When i am removing the ConvertDataFrameToValueFields() method i gain the missing time i need to fulfill the project.

Is there a way to improve the whole thing? Problem is that the idea has to be as it is. In the ConvertDataFrameToValueFields() method i have to map the returning DataFrame, the DataBytes array, to the MessageValueField properties of the specific class. Im getting all containing messagevaluefields by calling the ConvertDataFrameToValueFields().

Edit #1

Some further Info: The ConvertDataFrameToValueFields Method is called several times in the program life cycle. I have an object which contains at first the DataBytes array. Now i have to map the values of the DataBytes array to the specific four properties. this in the ConvertDataFrameToValueFields method points not always the same type of class! It can be type of ClassA or ClassB or ClassC. ClassA, ClassB and ClassC have different properties with the custom attribute IsValueField. Thats why i call determineCanMessageValueFields. I need to get the four specific properties, get the type of the property, and set the value based on the property type. I do this several times in the program life cycle and this is not always the same type. I hope this makes it a little bit clearer.

In Short:

DataBytes array has been filled --> Get the message value fields of the current object the DataBytes have been filled --> Determine the type of the specific property --> Set the value with the specific DataByte value(s)

\$\endgroup\$

migrated from stackoverflow.com May 9 '14 at 14:52

This question came from our site for professional and enthusiast programmers.

  • 2
    \$\begingroup\$ Check this out \$\endgroup\$ – Sriram Sakthivel May 7 '14 at 13:08
  • 1
    \$\begingroup\$ The NotImplementedException is misleading. ToType should throw a NotSupportedException for a typeof(BitmaskValueFieldClass). \$\endgroup\$ – Mathieu Guindon May 9 '14 at 14:55
  • \$\begingroup\$ Thanks for the input, but changing the type of exception does not lead to a performance upgrade. \$\endgroup\$ – Daniel W. May 12 '14 at 7:05
  • \$\begingroup\$ What happens, when result.size() > 4? Your code probably would break with an IndexOutOfRangeException. \$\endgroup\$ – Vogel612 May 12 '14 at 7:48
  • \$\begingroup\$ Could you put the whole class for review? It would be great to see the properties you have in order to understand better. Because right now I have a hard time understanding why your class would use reflection on itself. \$\endgroup\$ – IEatBagels Oct 7 '14 at 13:19
2
\$\begingroup\$

first of all reflection is expensive when used often. What you can do is

  • cache the properties to set

    Dictionary<Type, IEnumerable<PropertyInfo>> _propertiesCache
    
  • remove the copying over by creating the result immediatly and let it parse it itself

    public TMessage ConvertDataFrameToValueFields<TMessage>(byte[] dataBytes, Whatever unitType) where TMessage : new()
    {
        var result = new TMessage();
    
        ...
        // use the constructor which takes a ushort and the propertyName. The class itself has to know how to interpret the ushort
        fieldValue = Activator.CreateInstance(prop.PropertyType, value, prop.Name);
       ...
    }
    
  • limit the calls to GetCustomAttributes by using the result twice

    var isValueField = prop.GetCustomAttributes(false).OfType<IsValueFieldAttribute>().FirstOrDefault();
    if (isValueField != null)
        results.Add(isValueField.FieldIndex, prop);
    
  • remove the conversion of byte[] to List because it is not needed. Convert takes the startindex anyway

  • remove the need to store the bytes in the class

and together it will be

class ByteConverter
{
    private readonly Dictionary<Type, IEnumerable<PropertyInfo>> _propertiesCache = new Dictionary<Type, IEnumerable<PropertyInfo>>();

    public TMessage ConvertDataFrameToValueFields<TMessage>(byte[] dataBytes, Whatever unitType) where TMessage : new()
    {
        var result = new TMessage();
        int i = 0;
        foreach (var prop in GetMessageValueFields<TMessage>())
        {
            var value = BitConverter.ToUInt16(dataBytes, i);
            object fieldValue = null;
            if (prop.PropertyType == typeof(MessageStatusValueField))
            {
                fieldValue = new MessageStatusValueField(value, prop.Name, this.UnitType);
            }
            else if (prop.PropertyType == typeof(MessageValueField))
            {
                fieldValue = new MessageValueField(value, prop.Name);
            }
            else if (prop.PropertyType == typeof(MessageValueFieldInt32))
            {
                fieldValue = new MessageValueFieldInt32(value, prop.Name);
            }
            else if (prop.PropertyType.IsSubclassOf(typeof(BitmaskValueFieldClass)))
            {
                // use the constructor which takes a ushort and the propertyName. The class itself has to know how to interpret the ushort
                fieldValue = Activator.CreateInstance(prop.PropertyType, value, prop.Name);
            }

            if (fieldValue != null)
                prop.SetValue(result, fieldValue, null);

            i += 2;
        }
        return result;
    }

    private IEnumerable<PropertyInfo> GetMessageValueFields<TMessage>()
    {
        var type = typeof(TMessage);
        IEnumerable<PropertyInfo> properties;
        if (!_propertiesCache.TryGetValue(type, out properties))
        {
            //Get all properties with the 
            //custom attribute [IsValueField]
            var results = new SortedList<int, PropertyInfo>();
            foreach (var prop in type.GetProperties())
            {
                var isValueField = prop.GetCustomAttributes(false).OfType<IsValueFieldAttribute>().FirstOrDefault();
                if (isValueField != null)
                    results.Add(isValueField.FieldIndex, prop);

            }
            properties = results.Values;
            _propertiesCache.Add(type, properties);
        }
        return properties;
    }
}

remember to hold one global instance of this class or make everything static

you have not many Message classes and convert a lot you can also create Delegates to set the properties.

create to cache it

var message = Expression.Parameter(messageType, "message");
var value = Expression.Parameter(prop.PropertyType, "value");
var body = Expression.Assign(Expression.Property(message, prop), value);
var setProperty = Expression.Lambda(body, message, value).Compile();

and use it

setProperty.DynamicInvoke(message, fieldValue);
\$\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.