Which code you suggest?
-
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
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.)
Quad skating his way through the world since the early 80's... Booger Mobile - My bright green 1964 Ford Falcon - check out the blog here!! | If you feel generous - make a donation to Camp Quality!!
-
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")
-
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 don't want to see any of these running, because both are bad style. Inline-Statements, no brackets, bad string comparison. They are EVIL! :omg:
SharePoint Consultant and Developer at acocon Author of Primary ROleplaying SysTem War doesn't determine who's right. War determines who's left.
-
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
-
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
On top of everything else, you're post isn't even appropriate for this forum!
A guide to posting questions on CodeProject[^]
Dave Kreskowiak -
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'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...
-
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.
