Reflection Optimization
-
Just found this gem in my own code :doh: Worst part is that this runs quite often in tight loops in production code, haha.
public class SomeClass()
{
private PropertyInfo _propertyInfo;public SomeClass(PropertyInfo propertyInfo) { \_propertyInfo = propertyInfo; } public void ProcessStuff(object obj, object value) { // \[Lots of stuff omitted\] obj.GetType().GetProperty(\_propertyInfo.Name).SetValue(obj, value, null); }}
Who can spot the blunder?
Wow!!!
obj.GetType().GetProperty(_propertyInfo.Name)
LOL :-D But it might be useful, if you want to set a value on a different type that has a property with same name!! otherwise :mad:
-
Wow!!!
obj.GetType().GetProperty(_propertyInfo.Name)
LOL :-D But it might be useful, if you want to set a value on a different type that has a property with same name!! otherwise :mad:
Haha, I guess that might be an unlikely scenario in some weird situation, but not the case here unfortunately lol, just pure sillyness :)
-
Just found this gem in my own code :doh: Worst part is that this runs quite often in tight loops in production code, haha.
public class SomeClass()
{
private PropertyInfo _propertyInfo;public SomeClass(PropertyInfo propertyInfo) { \_propertyInfo = propertyInfo; } public void ProcessStuff(object obj, object value) { // \[Lots of stuff omitted\] obj.GetType().GetProperty(\_propertyInfo.Name).SetValue(obj, value, null); }}
Who can spot the blunder?
If he has several classes with common properties or methods, then why does he not simply use an interface and pass objects that inherit from that interface? Reflection can be very useful at times, but it should not be misused to work around flaws in the class design. Edit: And yes, that code must fall on its nose with an exception when objects are passed which do not have the property he's looking for. It's also not assured that _propertyInfo has ever been set, so a null reference is also possible.
At least artificial intelligence already is superior to natural stupidity
-
If he has several classes with common properties or methods, then why does he not simply use an interface and pass objects that inherit from that interface? Reflection can be very useful at times, but it should not be misused to work around flaws in the class design. Edit: And yes, that code must fall on its nose with an exception when objects are passed which do not have the property he's looking for. It's also not assured that _propertyInfo has ever been set, so a null reference is also possible.
At least artificial intelligence already is superior to natural stupidity
Perhaps the classes aren't known at compile time? Or maybe you want to dynamically copy properties of one object to matching properties in another? I have no idea lol, but I'm sure there exists some sort of valid use case for code like that.
-
If he has several classes with common properties or methods, then why does he not simply use an interface and pass objects that inherit from that interface? Reflection can be very useful at times, but it should not be misused to work around flaws in the class design. Edit: And yes, that code must fall on its nose with an exception when objects are passed which do not have the property he's looking for. It's also not assured that _propertyInfo has ever been set, so a null reference is also possible.
At least artificial intelligence already is superior to natural stupidity
I can't tell if you picked up the silly mistake or not - I cut out all the noisy code (I.e. Null checking), the point was to show that I already have a PropertyInfo object but I'm needlessly looking it up again each time by doing GetProperty(_propertyInfo.Name).
-
I can't tell if you picked up the silly mistake or not - I cut out all the noisy code (I.e. Null checking), the point was to show that I already have a PropertyInfo object but I'm needlessly looking it up again each time by doing GetProperty(_propertyInfo.Name).
-
Perhaps the classes aren't known at compile time? Or maybe you want to dynamically copy properties of one object to matching properties in another? I have no idea lol, but I'm sure there exists some sort of valid use case for code like that.
There sure is use for this, if you take a look at what happens when objects are instantiated with XAML or when you do data binding to controls. Some of that code must look similar internally. Below he also wrote that checking for null references was removed to reduce the amount of code. Still, reflection is something that should be used sparingly. Reflection is slow and can kill your program's performance if you rely too much on it. Wise words from somebody who uses XAML as markup for UI styles and layouts and even to load scene elements for a 3D engine. :)
At least artificial intelligence already is superior to natural stupidity
-
I can't tell if you picked up the silly mistake or not - I cut out all the noisy code (I.e. Null checking), the point was to show that I already have a PropertyInfo object but I'm needlessly looking it up again each time by doing GetProperty(_propertyInfo.Name).
That depends very much on your goals. This looks like you wanted to be able to assign any data to any object's property. In that case you would have to check wether the object in question really has the property you were looking for. I would expect something similar to this in the code that loads XAML markup and then creates object instances from it. It obviously must be able to assign values to any of an object's properties. If that kind of general purpose use was not intended, then I would not call your little mistake a code horror. It can be changed and optimized easily enough. Personally, I would be more concerned about two things: Performance and type safety. Reflection is slow and can kill your code's performance if you use it carelessly all over the place. That's why I would like to have all of it hidden away in some class and separated from the rest of the application. Then performance issues can be tracked down easily to the places where those classes are used, should they arise. Also, an application that passes around objects and relies on reflection in each and every method really would be a horror. All the checking that would be needed would be error prone and obfuscate the real intention behind each method. Debugging such a thing would be a pain and maintaining it a nightmare. That's why I again would prefer to use type safety to my advantage as much as possible and use reflection as sparingly as possible. And I would again hide it away in some class, so that the application stays clean of such generic and complicated code.
At least artificial intelligence already is superior to natural stupidity
-
I can't tell if you picked up the silly mistake or not - I cut out all the noisy code (I.e. Null checking), the point was to show that I already have a PropertyInfo object but I'm needlessly looking it up again each time by doing GetProperty(_propertyInfo.Name).
-
Just found this gem in my own code :doh: Worst part is that this runs quite often in tight loops in production code, haha.
public class SomeClass()
{
private PropertyInfo _propertyInfo;public SomeClass(PropertyInfo propertyInfo) { \_propertyInfo = propertyInfo; } public void ProcessStuff(object obj, object value) { // \[Lots of stuff omitted\] obj.GetType().GetProperty(\_propertyInfo.Name).SetValue(obj, value, null); }}
Who can spot the blunder?
Nice way of redundantly using property info.
-
So, you changed the "object obj" parameter to "SomeTypeWithPropertyInfo obj"? and replaced the shamed line with... obj._propertyInfo = value; This seems to be what you are implying in your hint...
No, the point is I have a cached instance of PropertyInfo that I'm not really using.
-
That depends very much on your goals. This looks like you wanted to be able to assign any data to any object's property. In that case you would have to check wether the object in question really has the property you were looking for. I would expect something similar to this in the code that loads XAML markup and then creates object instances from it. It obviously must be able to assign values to any of an object's properties. If that kind of general purpose use was not intended, then I would not call your little mistake a code horror. It can be changed and optimized easily enough. Personally, I would be more concerned about two things: Performance and type safety. Reflection is slow and can kill your code's performance if you use it carelessly all over the place. That's why I would like to have all of it hidden away in some class and separated from the rest of the application. Then performance issues can be tracked down easily to the places where those classes are used, should they arise. Also, an application that passes around objects and relies on reflection in each and every method really would be a horror. All the checking that would be needed would be error prone and obfuscate the real intention behind each method. Debugging such a thing would be a pain and maintaining it a nightmare. That's why I again would prefer to use type safety to my advantage as much as possible and use reflection as sparingly as possible. And I would again hide it away in some class, so that the application stays clean of such generic and complicated code.
At least artificial intelligence already is superior to natural stupidity
It's really not that complicated, I'm simply trying to show that the cached instance of PropertyInfo isn't really used.
-
I can't tell if you picked up the silly mistake or not - I cut out all the noisy code (I.e. Null checking), the point was to show that I already have a PropertyInfo object but I'm needlessly looking it up again each time by doing GetProperty(_propertyInfo.Name).
Yes, this is duplicate effort, but not if you have two object types with properties of the same name and you want the value of one object's property based on the property name of the other. Not that I can see a reason for doing this... I also read posts about how you take great performance hits using reflection, but this is often a non-issue. Many programs I write process at the speed that the user types, and reflection works faster than they do. Reflection has made my life dramatically easier and enabled me to create needed programs with a fraction of the code.
Brian Payne
-
There sure is use for this, if you take a look at what happens when objects are instantiated with XAML or when you do data binding to controls. Some of that code must look similar internally. Below he also wrote that checking for null references was removed to reduce the amount of code. Still, reflection is something that should be used sparingly. Reflection is slow and can kill your program's performance if you rely too much on it. Wise words from somebody who uses XAML as markup for UI styles and layouts and even to load scene elements for a 3D engine. :)
At least artificial intelligence already is superior to natural stupidity
Countered the univote. (I couldn't see why this was downvoted)
public class SysAdmin : Employee
{public override void DoWork(IWorkItem workItem) { if (workItem.User.Type == UserType.NoLearn){ throw new NoIWillNotFixYourComputerException(new Luser(workItem.User)); }else{ base.DoWork(workItem); } }}
-
Countered the univote. (I couldn't see why this was downvoted)
public class SysAdmin : Employee
{public override void DoWork(IWorkItem workItem) { if (workItem.User.Type == UserType.NoLearn){ throw new NoIWillNotFixYourComputerException(new Luser(workItem.User)); }else{ base.DoWork(workItem); } }}