Bet this algorithm never occured to anyone...
-
Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
bool CReportMethods::IsSalesTypeValid(const CXmlEntry & Entry)
{
if ( Entry.sty.GetValueAsLong() == 4L )
{
return(true);
}
else if ( Entry.sty.GetValueAsLong() == 5L )
{
return(true);
}
else
return(true);
} -
It probably depends on where you work and if you're the one who has to support the code in the future. If I was the one who had the support the code, I'd change it. At least then if something breaks, I'm also the one who has to fix it.
If it was my code, my responsibility etc, so would I. I have worked as a contractor for 25 years, and I can assure you it would NOT be changed at many places where I have worked without an RFC (request for change), cost benefit analysis, documentation, code review, test plan, version control, comment out old code and insert new, etc etc. However, if you are changing a part of the system which incorporates that particular bit of code then kill it, cus you have to do all of the above anyway. However, sometimes code you think does nothing, may do something. A slight change to the above algorithm, say that it becomes possible to drop through the code and exit without returning a return value. This throws an exception that is caught elsewhere. (That, for example, would happen with Powerbuilder 8+). Also, there may be a difference in behaviour between just returning true and looking at a passed invalid parameter (say a null pointer). For these sort of reasons, many sites will use an "if it ain't broke then don't fix it" approach. Chaning code withour permission is loose-cannon behaviour. I am not trying to support this position, all the fibres of my soul want to change code when I see it is badly written, designed or implemented. You just have to learn to sit on your coding hands.
Paul
-
-irregardless- :laugh: That's a good one for this discussion. I bet if you changed that to simply return true all the time, it would totally break the application...
"Quality Software since 1983!"
http://www.smoothjazzy.com/ - see the "Programming" section for freeware tools and articles. -
I can see you have been in the industry a while - I agree. I can think of at least 4 reasons why the code above may behave differently to "return true", depending on the language and hardware.
Paul
I'm wondering if the function that returns a 'long', actually does some kind of work behind the scenes. It's not good style, but it's not unheard of. It's a good one for The Daily WTF (I reject the new name of that site) And yeah I've been around a while... took a few years off to get a Biology degree, but it didn't do me much good. I went right back into this industry, but I found it much easier to get a job after college.
"Quality Software since 1983!"
http://www.smoothjazzy.com/ - see the "Programming" section for freeware tools and articles. -
I'm wondering if the function that returns a 'long', actually does some kind of work behind the scenes. It's not good style, but it's not unheard of. It's a good one for The Daily WTF (I reject the new name of that site) And yeah I've been around a while... took a few years off to get a Biology degree, but it didn't do me much good. I went right back into this industry, but I found it much easier to get a job after college.
"Quality Software since 1983!"
http://www.smoothjazzy.com/ - see the "Programming" section for freeware tools and articles.Jasmine2501 wrote:
I'm wondering if the function that returns a 'long', actually does some kind of work behind the scenes.
:omg: And this function is then called once or twice? And expected to not have side effects? If that is the case, someone needs a serious spanking (maybe me:cool:) You simply do not code that way. And newbies have the right to be educated properly! Thats their supervisors responsibilities!
Failure is not an option - it's built right in.
-
Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
bool CReportMethods::IsSalesTypeValid(const CXmlEntry & Entry)
{
if ( Entry.sty.GetValueAsLong() == 4L )
{
return(true);
}
else if ( Entry.sty.GetValueAsLong() == 5L )
{
return(true);
}
else
return(true);
}Placeholder where they never put the live code in?
-
Jasmine2501 wrote:
I'm wondering if the function that returns a 'long', actually does some kind of work behind the scenes.
:omg: And this function is then called once or twice? And expected to not have side effects? If that is the case, someone needs a serious spanking (maybe me:cool:) You simply do not code that way. And newbies have the right to be educated properly! Thats their supervisors responsibilities!
Failure is not an option - it's built right in.
Yeah not good. I always look at the code my guys are doing. In a job ad we recently advertised that you can learn on the job with an experienced mentor. We still didn't find anybody :)
"Quality Software since 1983!"
http://www.smoothjazzy.com/ - see the "Programming" section for freeware tools and articles. -
If it was my code, my responsibility etc, so would I. I have worked as a contractor for 25 years, and I can assure you it would NOT be changed at many places where I have worked without an RFC (request for change), cost benefit analysis, documentation, code review, test plan, version control, comment out old code and insert new, etc etc. However, if you are changing a part of the system which incorporates that particular bit of code then kill it, cus you have to do all of the above anyway. However, sometimes code you think does nothing, may do something. A slight change to the above algorithm, say that it becomes possible to drop through the code and exit without returning a return value. This throws an exception that is caught elsewhere. (That, for example, would happen with Powerbuilder 8+). Also, there may be a difference in behaviour between just returning true and looking at a passed invalid parameter (say a null pointer). For these sort of reasons, many sites will use an "if it ain't broke then don't fix it" approach. Chaning code withour permission is loose-cannon behaviour. I am not trying to support this position, all the fibres of my soul want to change code when I see it is badly written, designed or implemented. You just have to learn to sit on your coding hands.
Paul
Wow, I must say I am still learning the ways of software development but your post is interesting and enlightening in so many ways. Very interesting to try and understand all the ramifications of changing even the smallest and seemingly insignificant piece of code in an already developed project. I suppose to most sobering fact is that there are probably many cases where you just have to put up with bad code purely due to the fact that it is just not cost effective to go and correct the mistake/s.
-
yeah, but its a function that returns true no mater what the conditions are, and judging by the calls it makes, doesnt do anything else.so..
:badger:
-
to_be_defined wrote:
Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
Hold on, so what you're saying is that you keep looking at a function that does a few checks and returns (true) irregardless of what it checks and what happens.....and _no-one_ has changed it to remove the checks...hmm :\
:badger:
-
Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
bool CReportMethods::IsSalesTypeValid(const CXmlEntry & Entry)
{
if ( Entry.sty.GetValueAsLong() == 4L )
{
return(true);
}
else if ( Entry.sty.GetValueAsLong() == 5L )
{
return(true);
}
else
return(true);
}to_be_defined wrote:
Every time I read this I get an overwhelming feeling of wonder...
All sales types are valid because the customer is always right. ;P
_________________________ Asu no koto o ieba, tenjo de nezumi ga warau. Talk about things of tomorrow and the mice in the ceiling laugh. (Japanese Proverb)
-
If it was my code, my responsibility etc, so would I. I have worked as a contractor for 25 years, and I can assure you it would NOT be changed at many places where I have worked without an RFC (request for change), cost benefit analysis, documentation, code review, test plan, version control, comment out old code and insert new, etc etc. However, if you are changing a part of the system which incorporates that particular bit of code then kill it, cus you have to do all of the above anyway. However, sometimes code you think does nothing, may do something. A slight change to the above algorithm, say that it becomes possible to drop through the code and exit without returning a return value. This throws an exception that is caught elsewhere. (That, for example, would happen with Powerbuilder 8+). Also, there may be a difference in behaviour between just returning true and looking at a passed invalid parameter (say a null pointer). For these sort of reasons, many sites will use an "if it ain't broke then don't fix it" approach. Chaning code withour permission is loose-cannon behaviour. I am not trying to support this position, all the fibres of my soul want to change code when I see it is badly written, designed or implemented. You just have to learn to sit on your coding hands.
Paul
Your point is well taken. I can see where a change to this code as innocent as it sounds could cause a large problem for everyone. This without doubt explains why software created by the giants frequently breaks. The Coders don't want to go through the mountain of $xxt and paperwork it would take to fix a seemingly minor problem. And so we go back to a time when we put in NOPs into ASM code to later take it out and call it an update.
-
Here's one of my favourite functions in a system I'm working on right now. Every time I read this I get an overwhelming feeling of wonder...
bool CReportMethods::IsSalesTypeValid(const CXmlEntry & Entry)
{
if ( Entry.sty.GetValueAsLong() == 4L )
{
return(true);
}
else if ( Entry.sty.GetValueAsLong() == 5L )
{
return(true);
}
else
return(true);
}Slick. Now if I could only hack into my girlfriend's brain and override her IsHorny function with this algo, life would be good.
"Half this game is ninety percent mental." - Yogi Berra If you can read thank a teacher, if you can read in English, thank a Marine.