Which code you suggest?
-
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 doThe 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)
-
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 doThe 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.'"
-
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)
-
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.
-
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)
-
I'm a fan of early exit, so I'd go with the first one (assuming that's what this post is about). It doesn't really matter much in smaller routines like the one above, but when you have longer ones it can be difficult to follow retValue around the function to find out where you are really setting the return value. The first method is shorter because you can run a case through in your head without having to write down variables. I think the question boils down to Early Exit versus Single Exit. There's a lot of debate on the merits of both so I think your answers are going to be somewhat distributed between them. I've yet to hear a debate for single exit that I agree with over the merits of early exit...
Quote:
it can be difficult to follow retValue around the function to find out where you are really setting the return value.
That's funny. That's the same reason people usually argue for single exit, in that it is hard to figure out why some code isn't running because it turns out there was a return statement earlier that you hadn't noticed. :) To each his own.
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)
-
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.
the 2 loops are a good example, but for me is easier to track multiple exit points than it is to track changes on a variable. there may be a hundred variables involved, the return value can get reset, millions of things can happen, but when there's a return statement, i know it's over. whatever i've at that point is the result, i can track individual cases one at a time. for me it's really easier to track execution paths this way, is it different for you? the ammount of different opnions when we talk about code is just funny ;P opnions are the oposite of highlanders, there can never be only one.
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.'"
-
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
First of all, the second code should have a 'break' as follows. Otherwise, it just continues to loop pointlessly, even if it find "ABC" in the first item.
Boolean DoSomething(string[] values)
{
bool retValue = false;
foreach (string s in values)
{
if (s == "ABC")
{
retValue=true;
break;
}
}
return retValue;
}Personally, I choose the this method over your first method, since it has a single point of return to the method. About the performance, both versions should be identical as this method would only execute 2 steps extra.
-
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
So, this is certainly choosing between the lesser of 2 evils :wtf: If I HAD to choose one of both, the first example would be it, even if only for performance reasons - since the loop breaks when ABC was found. But having said that... No. Just... No. :-D
-
The first will be faster, as it will exit as soon as finding "ABC" Alternatively,
return values.Contains("ABC")
-
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
Code 1 would be my preference as it is the default indentation model for Visual Studio. I don't see the point of re-setting the default model or turning off the auto-complete for the sake of a preference; I find that the usual combination of indents provides a readable and fast coding format which is pretty good at revealing errors and incomplete blocks due to items not aligning conventionally. In other words, you can pick up errors in peripheral vision, which is quick.
-
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)
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. ;P
-
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
I would go with #1. Number 2 should have a break statement and would work too.
Boolean DoSomething(string[] values)
{
bool retValue = false;
foreach (string s in values)
if (s == "ABC")
{
retValue=true;
break;
}
return retValue;
}The signature is in building process.. Please wait...
-
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. ;P
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)
-
Quote:
it can be difficult to follow retValue around the function to find out where you are really setting the return value.
That's funny. That's the same reason people usually argue for single exit, in that it is hard to figure out why some code isn't running because it turns out there was a return statement earlier that you hadn't noticed. :) To each his own.
There are only 10 types of people in the world, those who understand binary and those who don't.
I also go mostly for early exit. This helps to keep the code to the left as much as possible. Besides. if a method gets too long... you're doing it wrong ;)
-
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)
-
Interestingly, the only use MS recommends for
gotois exiting multiple loops.Yes, but MS recommends you install Windows 8 on your computer...
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
