|
Sadly, I have seen too much code like that. Having a useless message is marginally more useful than having nothing there. 
Just because the code works, it doesn't mean that it is good code.
|
|
|
|
|
In my C/C++ code I often write similar things: assert statements. ASSERT -> "It should never happen that the specified expression evaluates to false". Of course ASSERT is usually compiled only into some non-release configurations. I use different kind of assert statements, I call one of these the "This should never happen" assert: This is an assert that always fires unconditionally with a specified message when executed. For example assert_message("Unhandled EMode enum member");
enum EMode
{
EMode_mode1,
EMode_mode3,
};
void Func(EMode mode)
{
switch (mode)
{
case EMode_mode1:
break;
case EMode_mode2:
break;
default:
assert_message("Unhandled EMode enum member!");
break;
}
}
If someone extends the enum with a new member (and lets say the functions that use the enum are not next to the enum in your files) and some codepieces are not updated along with the enum then at least you get a runtime error sooner or later in non-release builds. Of course there are a lot of other places where "This should never happen" asserts are useful.
|
|
|
|
|
Code1:
Boolean DoSomething(string[] values)
{
foreach (string s in values)
if (s == "ABC")
return true;
return false;
}
Code2:
Boolean DoSomething(string[] values)
{
bool retValue = false;
foreach (string s in values)
if (s == "ABC")
retValue=true;
return retValue;
}
in the above 2 codes which code you will suggest and why? waiting for your valuable comments.
Thanks
--RA
|
|
|
|
|
The first will be faster, as it will exit as soon as finding "ABC"
Alternatively,
return values.Contains("ABC")
|
|
|
|
|
You left out the ;
-= Reelix =-
|
|
|
|
|
Reelix wrote: You left out the ;
Well, the benefit of your first option is that it is faster as has already been mentioned. Sometimes built-in's are faster.
For that reason you might want to look at the built-in function. I read that someone thought the bubble sort put in an article was very efficient. I'm going "Oh G.., save us from inexperienced programmers" Built the bubble sort, a slightly more efficient version, and my binary sort routine I (re)wrote after seeing that #%#$@. I stopped testing performance of the bubble sort at 200K (over 2 minutes) I threw in the built in sort routine too. Both were sorting 200K in sub-second times. After getting up there in size, the built-in was performing in about 2/3 the time my routine was.
In Big O, the bubble was N^2 and time tests matched that estimated. I stopped testing mine at 150M (space ran out at 200M) Built-in 29 seconds, mine 49 seconds, slightly faster than 2/3. I estimated the bubble would finish in 750 squared times 130 seconds.
|
|
|
|
|
Personally I prefer the second option, as there's only a single return from the function.
That said, there's a whole bunch wrong with the code... or at least not good practice. (Yes, I realise it's just an example.)
|
|
|
|
|
I don't want to see any of these running, because both are bad style. Inline-Statements, no brackets, bad string comparison. They are EVIL! 
|
|
|
|
|
I prefer the second method. I try not to have multiple places where a function can return.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
I think that's an unnecessary restriction - I prefer to do all my validation code / user notification en mass at the top of a method, and exit immediately.
if (!userGotHisNameRight)
Report It
return
if (!userManganagedHisAddressOK)
Report It
return
if (...)
...
Actual work the method is supposed to do
The alternataive being:
if (!userGotHisNameRight)
Report It
else if (!userManganagedHisAddressOK)
Report It
else if (...)
...
else
{
Actual work the method is supposed to do
}
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
|
|
|
|
|
Ya, I know. Some people like it and some don't. Generally speaking the amount of time saved by returning in various places is negligible.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
ryanb31 wrote: Generally speaking the amount of time saved by returning in various places is negligible.
Generalization is Bad. what would you say if there was a database access on that loop? or a file read/write? a service call? are those so rare that they can't be considered "general"?
I'm ok with having one single return point, provided that the method doesn't do any significant work. the general rule should be to exit as soon as you finish the work or realize there's nothing more to be done.
I'm brazilian and english (well, human languages in general) aren't my best skill, so, sorry by my english. (if you want we can speak in C# or VB.Net =p)
"Given the chance I'd rather work smart than work hard." - PHS241
"'Sophisticated platform' typically means 'I have no idea how it works.'"
|
|
|
|
|
But you're missing the fact that you can exit a loop when you find what you need. You don't have to continue processing.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
What if you have two nested loops? It's a lot harder to exit them both...
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
|
|
|
|
|
Quote: What if you have two nested loops?
You can't get out of the matrix. 
I didn't say there was never a reason for returning from multiple places. I just said I prefer not to.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
Trust me, I took the red pill a loooong time ago!
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
|
|
|
|
|
|
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
No. Actually it isn't. All you have to do (hush my mouth) is add a label to your single return point at the bottom of your procedure, and then (and I can't believe I'm saying this in open forum) "goto" that label. Simples!
Yes - I am more than old enough to know better but I do still use goto from time to time and I'm not totally averse to the odd setjmp/longjmp pair in my code. 
|
|
|
|
|
Or use a return, which is cleaner, and a lot more obvious...and won't get you strung up by the "goto is evil" lynch mob. And in this case it would be a horrible and unnecessary use of goto which would probably be deserving of the hempen necktie!
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
|
|
|
|
|
|
Yes, but MS recommends you install Windows 8 on your computer...
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
|
|
|
|
|
No no, it's Windows 8.1 now...
At least they're bright enough to realise 8 -> 8.1 should be a free upgrade.
|
|
|
|
|
Easy: introduce a flag indicating when you're done processing (for whatever reason). You can either add that flag to the breaking condition of control statements, or add a single additional nesting layer inquiring the state of that flag everytime you're about to do some more processing (that could be skipped).
I've been using that concept successfully for a long time in legacy applications that have lots of very long functions with very high nesting levels. This method at most adds one nesting level, if at all, and it helps keeping track of stuff I need to clean up at various nesting levels before actually exiting the function.
|
|
|
|
|
Yes you can, but...a return is a lot, lot cleaner!
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
|
|
|
|
|
... until you introduce code that needs clean-up at one point or another.
Many of the functions I look at every day are a decade old or more, and consist of several hundred lines of codes with half a dozen levels of nesting or more. Every single one of them allocates stuff, or does something else requiring cleanup. More often than not, this happens before something else happens that necessitates a premature return. Some of the really old functions use goto exit; to immediately jump to the cleanup code. I use a flag.
Sure, not everyone works on such a codebase. But the truth is, the majority of programmers doesn't work on brand-new projects either. 80% work on internal programs designed to improve certain processes inside of a single company. Lots of code, and sometimes with a lifetime higher than that of some of their current programmers. In that context, IME, premature returns are almost always a bad idea.
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
