|
Thanks. Who knows, but probably it was just an accident. When someone really disagrees it would be more interesting to read his reply.
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).
|
|
|
|
|
You must have done it because reflection is really cool, right? 
I did know a coder who did that, he didn't realize that there was a serious performance penalty. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
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.
|
|
|
|
|
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.
|
|
|
|
|
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
|
|
|
|
|
Nice way of redundantly using property info.
|
|
|
|
|
I was reading some of the legacy code that is being used by one of our applications. there are some .js files which are meant to be used by new developments so that we dont have to "reinvent the wheel" (thats what we have been told)
I opened the first file and first function and i saw
function somefucntion()
{
.
.
.
if (!flag)
{
retVal = false;
return false;
}
retVal = true;
return true;
}
What the heck? Is this some kind of javascript return value optimization that I am unaware of because i think i can simply replace these lines with return flag;
P.S. even after pointing in out I am not allowed to change is as we are not allowed to change this code as "it might break something" (WT Elephant?)
|
|
|
|
|
Rahul Rajat Singh wrote: "it might break something" (WT Elephant?)
Yes it might break original coder's heart 
I once saw this in production code:
bool func(bool check)
{
if(check)
{
if(check)
{
}
}
}
|
|
|
|
|
The comment is wrong: It must be 200%
|
|
|
|
|
yes indeed ![Rose | [Rose]](https://codeproject.global.ssl.fastly.net/script/Forums/Images/rose.gif)
|
|
|
|
|
I guess this coder had seen the double check synchronisation pattern, e.g.
X getSynchronisedSingleCopyOfX(){
if(x == null){
lock(this){
if(x == null) x = new X();
}
}
return x;
}
... (commonly used in singleton instantiation) and didn't understand what the real benefit of the double check in that case was.
Or maybe he was just an idiot 
|
|
|
|
|
haah ai was laughing for about a minute, i remember now i was programming in c++ and i received the following message:"suspicious pointer conversion"
|
|
|
|
|
It can literally be replaced by 'return retval = flag'. If retval isn't local, changing it to 'return flag' might break something, though it's awful code if that's the case.
|
|
|
|
|
but the better code would be
if(flag) //skipping creating the ! result
return true;
return false;
[edit]
i thought it is better  ))))))))))))))))))))))))))))))))))))
[/edit]
modified 17-May-12 7:55am.
|
|
|
|
|
No, "return flag" is better than this. More words is worse if they add nothing.
|
|
|
|
|
Actually,
return flag could be wrong. flag could be a non-Boolean value (e.g. 0 or "" or [not so well known so a major gotcha] '\n' which have a Boolean equivalent of false; whereas most other values have a Boolean equivalent of true). So, to return false for ! flag and true for flag, you should use
return ! ! flag; . This is a useful trick: the 2nd
! converts flag (in this case) to a Boolean and negates it (true -> false and false -> true) then the 1st
! negates it back to what its Boolean equivalent was.
|
|
|
|
|
Good point, although using a variable name of 'flag' for something which wasn't a boolean would be deserving of headline mention in this forum!
Can't you just cast to boolean instead of using !!?
|
|
|
|
|
BobJanova wrote: Can't you just cast to boolean instead of using !!?
Yes, you could. Casting could be done using
return Boolean(flag); Note that
return new Boolean(flag); would return a Boolean object whereas without the new keyword, you'd be casting to a Boolean primitive. Greater clarity could be effected by using the triadic operator:
return flag ? true : false; or you could use short circuiting, viz
return flag && true; but using ! ! flag is more succinct.
(I noticed yesterday a piece of jQuery that used !1 for false - I wouldn't go that far in obfuscation).
|
|
|
|
|
IMHO, return flag; is still better than this.
|
|
|
|
|
oh f***, i didnt notice the stupidity )))) sorry guys
|
|
|
|
|
Its ok. If only this realization would have came to the original developer of this code
|
|
|
|
|
Yes, you must not change the sacred elephant code or you will anger the coding spirits. 
If they fear making changes to the code then they need unit tests to prove that the code still works after changes. Also, if the code is so brittle that it may break after a small improvement then it needs reviewed and replaced with better code.
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
