|
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.
|
|
|
|
|
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.
|
|
|
|
|
Why you think like that?
/* LIFE RUNS ON CODE */
|
|
|
|
|
If we ever get around to refactor this, then maybe that is the way to go. But not anytime soon. When I said 'really old', I meant it: some of that code predates exception handling by a decade.
Besides, there are plenty of good reasons not to use exceptions at every possible opportunity. E. g. I suppose you wouldn't suggest the use of exceptions in the case of the OP 
Flags (or states, if you prefer), are perfectly valid mechanisms for keeping track of the state of your processing. They're definitely not the only way to handle this, but there is no real downside to them either.
|
|
|
|
|
Flags and/or gotos are both reasonable approaches in languages that don't have a try...finally construct and I've written code using both approaches in many different languages. Since the OP was obviously C# I assumed that is what we were talking about, and in C# the try..finally (or the "using" construct, when applicable) is definitely the cleanest approach to making sure your resource cleanup happens, even when you don't think exceptions enter into the picture, though in my experience most cases where resource cleanup happens, exceptions at the .NET framework level are almost always a possibility.
And no I wouldn't suggest try..finally for the original post because no resource cleanup is involved.
|
|
|
|
|
Ok, lets forget language (old C) specific concerns: Assuming you have exceptions, yes, I agree that you can reasonably handle many cases of premature returns that way.
However, my take on exceptions is that code running as expected shouldn't throw one! Finding an element with specific properties in a container does not warrant throwing an exception, whether or not you need clean-up.
Look at the following code, ignoring language specific elements:
int find(Container c, double value, double tol)
{
int result = INVALID_ID; do_some_intialization();
for(size_t index = 0; index < c.size(); ++index)
{
if (element.distance(value) < tol)
{
result = element.ID();
}
}
do_some_clean_up();
return result;
}
There are various solutions to short-cut the loop once it finds a fitting element:
1. use some command that breaks out of the innermost loop (in C/C++ you can use break)
2. attach the check (result==INVALID_ID) to the loop header, so it quits once you assign a valid ID. (not sure how to do that with for_each in C#, but a standard for loop lets you add an arbitrary number of stop conditions easily)
3. Introduce a flag variable that indicates when you're done searching. As it would pretty much just store the current value of (result==INVALID_ID) in this case, you might as well go with solution 2 above
4. throwing an exception, catching it with try/finally to ensure proper cleaning up
In this example, my suggestion of introducing a flag variable turns out to be unneccesary, as the result variable itself can be used to pretty much the same effect. In my experience, this is often the case, so the effort to use these kind of checks instead of premature returns or gotos is often rather low.
Using an exception would certainly work, but it conflicts with my understanding of what an exception means. Also, if you do catch actual error cases with exceptions within that same code, you need to make sure to not catch the 'good-case-exceptions' as errors!
If you have no problem with that, the more power to you. But I prefer not to go that route.
|
|
|
|
|
I think you've completely misunderstood what I said. I am in no way advocating introducing new exceptions as a way of prematurely ending a loop. What I am advocating is using language constructs (try...finally or using(...)) specifically designed help ensure safe cleanup regardless of how you exit the function/procedure. It's simply cleaner and easier to get it right than using other methods, especially when there is the possibility of exceptions, which it turns out is almost always when using resources that need to be explicitly cleaned up in languages that provide those constructs.
|
|
|
|
|
Stefan_Lang wrote: premature returns are almost always a bad idea
Definitely a code smell.
|
|
|
|
|
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.
|
|
|
|
|
throw an exception 
To alcohol! The cause of, and solution to, all of life's problems - Homer Simpson
----
Our heads are round so our thoughts can change direction - Francis Picabia
|
|
|
|
|
|
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 
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.'"
|
|
|
|
|
Vague but true.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
Just because you can doesn't mean you should.
|
|
|
|
|
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.
|
|
|
|
|
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.
|
|
|
|
|
I do not see the value in always only having one return from a function. When reading the code, you still need to look through the function for every place retValue is set, so it is not easier to follow or understand. If there is some clean-up to be done, then it does make sense. But if not, why keep to this rule if it provides no benefit? In the validation example someone else posted in this thread, the early returns provide easier to read code, rather than a long block of if/then/else.
|
|
|
|
|
Quote: the early returns provide easier to read code
Easier to read is a matter of opinion. Some people think C# is easier to read than VB.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
ryanb31 wrote: I prefer the second method.
You can't be serious!
/ravi
|
|
|
|
|
Quote: You can't be serious!
I do like to joke around a lot, but yes, I can be serious at times.
There are only 10 types of people in the world, those who understand binary and those who don't.
|
|
|
|
|
So if the array contained a million strings, the first of which was "ABC", you'd check every single string even though you already know you have a match? 
/ravi
|
|
|
|
General 
News 
Suggestion 
Question 
Bug 
Answer 
Joke 
Praise 
Rant 
Admin 
Submit an article or tip
