Which code you suggest?
-
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)
Exactly. I share the same view - do validation and exit as soon as possible, to avoid crippling the logic down the road.
Interested in Machine Learning in .NET? Check the Accord.NET Framework. See also Sequence Classifiers in C# with Hidden Conditional Random Fields.
-
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)
OriginalGriff wrote:
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.
Exactly. I share the same view - do validation and exit as soon as possible, to avoid crippling the logic down the road.
Interested in Machine Learning in .NET? Check the Accord.NET Framework. See also Sequence Classifiers in C# with Hidden Conditional Random Fields.
-
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
If I were trying to determine if "ABC" exists in values then Code1. If I were trying to determine if the last element in values (s) was equal to "ABC" then Code2. I subscribe to early returns. Unless there is a reason to tenderize the value(s) before returning it.
-
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.
i don't think they count the parameter list, i've read that on a doc that came on a CP newsletter long ago, but i personally don't like that much parameters
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.'"
-
OriginalGriff wrote:
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.
Exactly. I share the same view - do validation and exit as soon as possible, to avoid crippling the logic down the road.
Interested in Machine Learning in .NET? Check the Accord.NET Framework. See also Sequence Classifiers in C# with Hidden Conditional Random Fields.
I totally agree with you guys. But its also important what your boss thins. For example when i used the same technique validations with fast returns i got scold how this shouldn't be done this way because when someone reads the code he wont understand a thing ?! And also this isn't a good practice ?!
Microsoft ... the only place where VARIANT_TRUE != true
-
I totally agree with you guys. But its also important what your boss thins. For example when i used the same technique validations with fast returns i got scold how this shouldn't be done this way because when someone reads the code he wont understand a thing ?! And also this isn't a good practice ?!
Microsoft ... the only place where VARIANT_TRUE != true
It seems a lot of people follow to this "rule" without ever questioning why. My manager was also an advocate of the "single return improves readability" fallacy until I explained to him why this wouldn't be the case with examples. Plus when I completely rewrote a ugly mass of haired code in our system into actually maintainable code, he was convinced. I think it needs some luck, and having open-minded management helps too.
Interested in Machine Learning in .NET? Check the Accord.NET Framework. See also Sequence Classifiers in C# with Hidden Conditional Random Fields.
-
... 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. -
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
Both and neither, i usually use what makes more sense in the task at hand, if the method is trivial or requires final cleanup, I prefer a single return point, if depending of a condition the code will run a lengthy or expensive task then I prefer early return.
CEO at: - Rafaga Systems - Para Facturas - Modern Components for the moment...
-
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
One way in, one way out and we stop when we find the value we want.
Boolean DoSomething(string[] values)
{
bool bResult = false;
int i = 0;
while ( (!bResult) && (i < values.GetLength()) )
{
bResult = (values[i++] == "ABC");
}
return bResult;
}OR
Boolean DoSomething(string[] values)
{
bool bResult = false;
int i = values.GetLength();
while ( !bResult && i )
{
bResult = (values[--i] == "ABC");
}
return bResult;
}Just realized with the my second one, if the array is empty it still works. The first one does too but I like the second better and will try using that logic from now on. Thanks for making me think.
M__k T J.hnnnnn What my signature looks like in courier.
-
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 prefer #3 offered by "NeverJustHere": return values.Contains("ABC"); Each will get the same result, except #2 wastes unnecessary cycles. #3 is best because it is the most efficient, uses the least about of your code, and is already debugged (hopefully MS did their job). The example in #2 can be made more efficient if you change it to: Boolean DoSomething(string[] values) { bool retValue = false; foreach (string s in values) if (s == "ABC") { retValue=true; break; } return retValue; }
-
Reelix wrote:
You left out the ;
:laugh: 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.
-
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
-
One way in, one way out and we stop when we find the value we want.
Boolean DoSomething(string[] values)
{
bool bResult = false;
int i = 0;
while ( (!bResult) && (i < values.GetLength()) )
{
bResult = (values[i++] == "ABC");
}
return bResult;
}OR
Boolean DoSomething(string[] values)
{
bool bResult = false;
int i = values.GetLength();
while ( !bResult && i )
{
bResult = (values[--i] == "ABC");
}
return bResult;
}Just realized with the my second one, if the array is empty it still works. The first one does too but I like the second better and will try using that logic from now on. Thanks for making me think.
M__k T J.hnnnnn What my signature looks like in courier.
-
One way in, one way out and we stop when we find the value we want.
Boolean DoSomething(string[] values)
{
bool bResult = false;
int i = 0;
while ( (!bResult) && (i < values.GetLength()) )
{
bResult = (values[i++] == "ABC");
}
return bResult;
}OR
Boolean DoSomething(string[] values)
{
bool bResult = false;
int i = values.GetLength();
while ( !bResult && i )
{
bResult = (values[--i] == "ABC");
}
return bResult;
}Just realized with the my second one, if the array is empty it still works. The first one does too but I like the second better and will try using that logic from now on. Thanks for making me think.
M__k T J.hnnnnn What my signature looks like in courier.
-
Yes you can, but...a return is a lot, lot cleaner!
The universe is composed of electrons, neutrons, protons and......morons. (ThePhantomUpvoter)
No, is not. This "one return only" style you talk about always produces uglier code, and is usually slower. This is clearly a personal style preference of yours and you are entitled to it, but if you think there is a general rule that recommends only one return then you should know you are wrong.
-
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.
The idea of adding an additional unnecessary variable for "control" is anathema to me. Think of it, you are wasting an assignation and then you are adding an extra comparison for each block that fails plus the one that's actually true. Put that into a function that repeat a few thousand times and you got yourself a waste of time well into the millisecond range if not more, and lot of extra work for the GC. I am not against flags, God knows i used them a lot, they are a valid control, but not in this case where you are using it as a sort of eufenism for a return
-
Stefan_Lang wrote:
Some of the really old functions use
goto exit;to immediately jump to the cleanup code. I use a flag.That's what try..finally is for. Both goto and flags fail miserably in the presence of exceptions.
-
Oh, you must be lots of fun to work with. The "correct code"? The "correct code" is something that is easy to maintain and produces the desired result. In addition your comment does not address the original question. Which is better, multiple returns or a single return? As a maintenance programmer, I fully support a single return statement. Multiple returns are lazy and prone to creating code that doesn't get tested before getting sent to QA. Granted the example was trivial but the underlying question was not.
-
The idea of adding an additional unnecessary variable for "control" is anathema to me. Think of it, you are wasting an assignation and then you are adding an extra comparison for each block that fails plus the one that's actually true. Put that into a function that repeat a few thousand times and you got yourself a waste of time well into the millisecond range if not more, and lot of extra work for the GC. I am not against flags, God knows i used them a lot, they are a valid control, but not in this case where you are using it as a sort of eufenism for a return
Renzo Ciafardone wrote:
unnecessary variable
YMMV. If the alternative is
goto, I choose the variable. If it is 15 layers of nested control statements, I choose the variable. If I know that anyone, including me, may be reading and trying to understand that code next month, I'm using a variable. I'm not saying to always use such a variable - only when it helps keeping the code clean and maintainable. IMO, the variable is sensible and necessary in these cases. (Also, these cases cover pretty much all the code I've worked on over the past 30 years)Renzo Ciafardone wrote:
you are wasting an assignation and then you are adding an extra comparison for each block that fails plus the one that's actually true
You are underestimating the efficiency of an optimizer: in most cases you won't even notice a difference, as the compiler will optimize away any variable that is only used sporadically. The only exception is if there are multiple checks at the top nesting layer: then you need to add one condition to each top level check after the one that contains the 'abort condition'. The alternative would be just one check and moving the rest of the code down one nesting level. The former may have a minor impact on performance (but see below), the latter will always adversely affect code complexity, and thereby the likelyhood of bugs and the effort of maintenance. Your choice. In the past, I've tended to the latter. But now that I have to deal with that same code, I've decided - for my own benefit - to use the former.
Renzo Ciafardone wrote:
you got yourself a waste of time well into the millisecond range if not more
I very much doubt that. More importantly, even if it were true for one in a thousand or even one in ten (meaningful!) applications, don't design and write code under the assumption of the worst possible effect on performance, write under the assumption that you need to maintain and rewrite code often! If you have an application with an expected lifetime measured in weeks rather than months. If that application is extremely performance-critical. If there is no meaningful numerical processing involved that may cause performance problems. If there is no external database, internet connection, file IO or just any IO involved. If you're using a compiler with a c
