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
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)
-
Yes, but MS recommends you install Windows 8 on your computer...
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
-
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
Neither. IMHO the best approach for a non-trivial function is to use a single return value declared and initialized at the start, along with a flag indicating premature abortion. Then, over the whole length of your function, no matter how long it is, you can always add that flag to every loop or (top level) if statement to prevent unnecessary execution of code. In the example above, the return value can double as abortion flag: use it to prematurely break out of the loop to prevent unnecessary execution of code. Personally, for any function with more than about half a dozen of control statements, I introduce a boolean variable called 'done' or similar that I use to short-cut later control statements. This way I don't normally need to increase the nesting level by more than 1.
-
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)
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.
-
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)
-
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
It depends. There are times when it is necessary to exit as soon as a decision is reached, as I expect the performance of foreach to be O(n). For example, if a function is trying to find the first prime number within an array then you need to exit as soon as the condition is true, otherwise much time may be wasted. In the example given it depends how large I expect the string array to be, clearly the function itself can potentially take a string array of any size so method 1 seems preferable to me. However, if you don't like multiple return points then use 'break'. A more complex function is often clearer to read and debug if there is only one return point, and this is preferable if speed is not an issue. For example, if a function decides which way a bug is going to turn - left or right, then it may test many conditions, none of which may involve operations on large collections. In this example, it is probably preferable to have only a single return point.
-
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 think an early exit is always best. If you don't have an early exit this can often lead to more nesting later on in the method to allow you to skip the rest of it and unessassary nesting leads to more complicated code. (You could argue that you should splt the method up in this case but I say keep logical units together.)
-
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 ;)
as per nasa specs, 60 lines on the most extreme atomic task, 15 for anything else.
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.'"
-
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.
yours is the most sensible way to handle a single exit.
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
Don't mind.
-
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.
he was talking about the exit model of the method, the indentation is just a incedent due to the html pasting.
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.'"
-
he was talking about the exit model of the method, the indentation is just a incedent due to the html pasting.
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.'"
He didn't talk about anything. He simply presented code and asked for preferences.
-
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. -
as per nasa specs, 60 lines on the most extreme atomic task, 15 for anything else.
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.'"
Quote:
15 for anything else.
Serious? I have lots of stored procedures that take 20 or more parameters so just adding the parameters is more code than 15 lines.
There are only 10 types of people in the world, those who understand binary and those who don't.
