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)
1 Answer 1
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);
NotImplementedException
is misleading.ToType
should throw aNotSupportedException
for atypeof(BitmaskValueFieldClass)
. \$\endgroup\$result.size() > 4
? Your code probably would break with anIndexOutOfRangeException
. \$\endgroup\$