|
At the risk of further baiting the troll, man you take this stuff WAY to seriously. "Garbage"? "Harebrained adherence"? "Not being familiar with the state of the art"? "But if you're writing crufty imperative code"? Dude, you need to get out more, seek some kind of relaxation therapy because you are headed for a coronary if you don't find an outlet for your anger.
Did I insult you? Maybe with the "joy to work with" comment, but really, your first statement was "I would never hire anyone . . ." Someone asked a question that related to opinion. I responded with my opinion, which is just as valid as every other opinion that has been posted in response to the original question.
Heaven forbid I assume anything about you but, how much maintenance programming have you done in your career. Real maintenance programming of a system you didn't start but was hired on well after version 1 or even 2 had shipped? In that world, you play with the hand you are dealt, and on more occasions than I care to remember I have had to deal with code that had NOT been tested before being sent to QA because the programmer just threw in multiple return statements because they ASSUMED they knew how the user would interact with the system. Code goes to the field and the user does something different than what the programmer thought the user would do and BOOM goes the code. The else condition was never tested because the programmer only tested with "good" entries.
It is a mistake we ALL make. We make this mistake because we write the code and know what it SHOULD do and get locked into the mindset that "the user will only try to enter the correct information like I have been testing because that what makes the code work and produce the result the users want." Negative testing and single exits can simplify the maintenance of the code long after the original developer has moved on.
You don't have the luxury of rewriting every module you come across to meet your coding standard, you have time to fix the bug and move on to the next one, after of course you thoroughly test your changes and make sure the changes you made didn't break something else. That something else might not even be in the file you are modifying.
So in conclusion, jibalt, you will find over time that life is too short to worry about this stuff and code is just code. Write it so others can see what is going on because in 6 months from now you will have a hard time remembering why you did what you did when you look back at something you thought was so elegant. That is competence AND experience talking.
Mark T Johnson
Old School Programming
K.I.S.S. because you will probably be the S. who has to go back into that file and fix something.
|
|
|
|
|
|
I debated last night when I saw your comment. Should I respond to this person who waited 5 months to flame me over a lengthy post about life being too short?
Then my curiosity got the better of me and I decided I had to find out why.
So, why, jibalt, why did it take you 5 months to respond to my post? Why after 5 months were you compelled to once again call me a pathetic hypocrite? Every post you have made towards me in this thread has been filled with venom and vitriol. What is it about me that brings out that much anger from you?
5 months. Sounds about right for a jail sentence. Is that where you've been? I can see you having road rage or being in a bar fight or beating up your significant other.
Oh, my blood pressure was quite normal when I wrote that last post. Because you see, I enjoy poking the troll.
|
|
|
|
|
I refuse to hire anyone who subscribes to that inane single exit nonsense. And that code is awful for other reasons too. The correct code is
return values.Any(s => s == "ABC") unless it can be proved to be a performance bottleneck.
|
|
|
|
|
That may be OK for C#; but how does it translate to other languages?
|
|
|
|
|
Modern languages these days have similar functional facilities ... this includes C++11, and even Java finally in Java 8. Others are Ruby, Python (although limited to one line), Scala, Haskell, Clojure of course ...
|
|
|
|
|
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;
}
-- modified 27-Jul-13 0:03am.
|
|
|
|
|
It's tragic that anyone would ask this, since the second chunk of code is so clearly inferior.
But I would of course write
return values.Any(s => s == "ABC")
|
|
|
|
|
You have posted in the wrong place. The guidelines say no programming questions . This forum is purely for amusement and discussions on code snippets. All actual programming questions will be removed.
|
|
|
|
|
We had this discussion quite recently.
In the example you posted, the second is objectively worse because it doesn't leave the loop at the first opportunity. If you are going to go for a single return point, you must put a break in there.
C# provides a simple way to do this general action of 'find the first that matches a condition': the Linq extension method Contains. If this code is C# 3.5 or later then you should use that: return values.Contains(s => s == "ABC"). Note that you can use this for any construction of this type.
If you're in an environment where you can't do that, I'd pick the first. As soon as you have control structures you can't assume top-to-bottom flow anyway, and you have to track execution paths to follow the code, so there's no good argument for not using return any more.
|
|
|
|
|
Obviously you missed a break in your 2nd method (or I hope so). If you do this, it's actually a hybrid between the 2 methods. A more "strictly" no-goto (even hidden as a break / premature return) would be as per MarkTJohnson's code samples - i.e. add a check in the loop's conditions to see if the state variable has changed.
As for those advocating premature returns, but (in the same breath) dissing goto: Such a return is very related to a goto, as is a break inside a loop. All 3 result in much the same assembly code, the only difference is return/break is safer to use than goto - since goto has more freedom to screw-up. It would be similar in stating that you should always use typed pointers, very true - but it's not as if an untyped pointer is actually "that" different, just possible to do more damage. Not to say that goto's should be used, just to point out that you're walking a fine line between a good idea and contradicting yourself.
Also I'll give you the benefit of the doubt and not try to encourage to use the Contains (or other similar stuff) instead of the entire function, as some have already pointed out. With the thinking that you used this as a sample to make a point, not a sample to take literally.
That said, I tend to find making the 2nd method work properly and efficiently becomes more code. And with more complex functions the extra code becomes exponentially more. Although I don't have trouble reading the principle itself (either one, as long as formatted so the state-set or returns are clearly highlighted, e.g. a blank line after each), I tend to try and write less code (if possible) - so I'd probably go with method 1 if no other reasons are apparent.
This particular thing (i.e. premature return vs state variable) I see as subjective in the same sense that some would like / understand recursive more than iterative loops (or visa versa). If all else is the same - i.e. no need for other stuff to happen too just before the return.
Where it does become a pain, is if your function is later extended. You might later add a further portion which should run even if the value is already found. Perhaps some side effect, like counting how many times a positive was returned through the object's life (sorry - stupid example, but it happens).
|
|
|
|
|
"As for those advocating premature returns, but (in the same breath) dissing goto: Such a return is very related to a goto, as is a break inside a loop."
No it isn't ... goto's are unstructured. This is programming technology and history 101 ... actually .001 ...
"All 3 result in much the same assembly code"
Irrelevant, and not true, or not predictably true.
|
|
|
|
|
Second method if security is a concern (methods like that are not vulnerable to timing attacks[^]); first in all other cases (the first method will always execute in N time, the second is O(N)).
He who asks a question is a fool for five minutes. He who does not ask a question remains a fool forever. [Chinese Proverb]
Jonathan C Dickinson (C# Software Engineer)
|
|
|
|
|
the first method will always execute in N time, the second is O(N)
There is no such thing as "N time". They're both O(N), but the average time for the first one is half that of the second one.
|
|
|
|
|
Neither; I prefer not to end a foreach when a for will work just as well (if not better).
bool result = false ;
for ( int i = 0 ; !result && i < values.Length ; i++ )
{
result = values [ i ] == "ABC" ;
}
return ( result ) ;
This could also lead to a discussion of ifless programming.
|
|
|
|
|
I guess, I would try something like:
Boolean DoSomething(string[] values)
{
bool retValue = false;
if (values != null)
for(int i = 0; i < values.Count(); i++)
{
if (values[i] == "ABC")
{
retValue = true;
i = values.Count();
}
}
return retValue;
}
I know the example is a simple loop, but thinking in maintenance and performance this will:
- Avoid the internal context and memory usage of a FOREACH (FOR is recommended when u do only 1 single access to the object[i])
- Use of a state variable for a return is recommended rater that having a lot of returns. (readability?)
- The Return in the for or foreach cause a BREAK, if I'm not wrong that was expensive in the past, not sure with modern languages.
|
|
|
|
|
Hm, interesting,
I can agree a Little on your first point - although this is no practical thinking for most .NET developers, because the "perfomance" impact is outweighted by more expressive code - I do express something when I write "foreach" (I express I want to do something FOR EACH element, don't depend on index or IList, just IEnumerable, I won't need an index, I don't need a break condition, ...). So the additional 2 context variables used internally by the foreach loop are fast earned back...
Point 2 will never be decided on "the Internet" - recommended? - not by me - it depends to much on overall style of the code (a lot of coders first evaluate all arguments and do early returns) and personal choice - and if you do "Microoptimazations" in .NET like you/I did in old c++ days, you should do early returns - so, readability (for you) or performance?
Point 3, though irrelevant for this discussion because this code should break on any found case (op alternatives are not equivalent), but I'm wondering why you think setting the variable to the break condition and evaluate it again is faster than a break statement? If this is any faster (I will test) this is intersting to know - it seems you are the master of "high perfomance super optimized .NET code" - but I'm not shure if .NET is the right realm for code which needs this kind of optimizations - shouldn't we do such perf. critical things in native code?
Kind regards Johannes
|
|
|
|
|
Good reply!, I already don't remember if the Break is expensive, hasn't google.... but I remember it was like using the old GOTO, developer just avoids it when possible.
Yes, if we just think in the original code example, there is like no discussion, the code is so simple that it will return with the first find, so no real issues.
But you have talk about something that I has been seen in the "modern" developers and languages (not sure if can apply to the original topic):
"it seems you are the master of ""high perfomance super optimized .NET code"" - but I'm not shure if .NET is the right realm for code which needs this kind of optimizations - shouldn't we do such perf. critical things in native code?"
With modern PCs, the increase of memory and processors speeds (also number of CPUS) developers are forgetting about performance, speed and memory issues, its like they don't remember that the memory is not infinite, and that with the speed of data increase the application can go from 1 minute process to 1 week in a couple of months... I'm no a Performance maniac, but I worked like 4 year in Windows Mobile (C#) and I learned how important it is, and in fact has been a good ADD ON in today projects, I can "easy-sly" understand where our current project will fall and in fact they has fail (I was totally ignored by no mobile developers, then they didn't toke my recommendation, 2 months later the application was failed with Out of memory, with Time out, bandwidth, channel amount of data, with tooo long process).. today my coworkers are taking care of it, but they learned in the wrong way, when it failed in Production.
I can say I'm a little worried about modern programing, because our Junior are not facing those Memory and Performance issues, even my Cell Phone have more CPUs and Memory than my 4 years old PC... then neither current mobile developers are facing performance issues until its too late and they will be the Senniors of tomorrow
|
|
|
|
|
Very good points. I feel we may have quite similar experiences. I also worked for the last 7 years on Windows CE and Embedded with .NET CF and native code. On Win CE 5 (which was a big step already) we had the 32 MB memory limit per process - How funny, that TODAY one of these old CE 5 devices "showed up" on my office-desk with a "pretty" .net machine-visualization on it - customer (programmer) is telling me that he ran into problems... Guess what? -> Out of Memory!
So I totally agree with your points about "Juniors" - And I think we as programmers, will face the same story again, after someone comes up with the idea of the next IThing and want's to play tetris on it.
When I was a C++ programmer and realized that I didn't do so much optimizing and inlinig and what not any more, I thought: "Finally we arrived where Memory and CPU perf. is "enough" for my use-cases". But when I startet my first embedded device project a few years later, it was the old story again...
So I never mean't that Performance/Memory consumption doesn't matter, and in reality I think about it on every line of code I write (a habit hard to discard if you started programming in a time where it was "expensive"). - Like you said: a good Add On even in todays projects.
But I think that today the "code" has to do so much more than just "crunch the numbers". We write code in patterns, abstractions, with test support, support for different plattforms or frameworks, for services, devices, serialization scenarios, for easy maintenance, for whatever is needed on the project at hand, and only sometimes there are performance or memory constraints now (when I started this was ALWAYS one of the most important things - "unreadable guru code for safing 2 bytes? - no problem").
It may be a project where RAD and easy "junior-flipping" is wanted - so go for .NET, easy constructs, no optimazitions where deep knowledge is needed (a case where perf. of foreach vs. for should not matter). But another time you will have todo something very "fast" or "cheap on memory", then comes the time to optimize algorithms and code for specific tasks. But if I'm on a CE device, found a perfomance/memory bottleneck, I would hardly think about optimizing the IL code - I'd go for a native solution (maybe C++) and use Interop to call it (if marshalling perfomance impact is not a problem). Because from my experience the "biggest" gain on using native code, is to circumvent the GC...
Of course using unsafe code can be an alternative, and optimizing perf/memory is the only good reason one may do it (apart from legacy code calling needs)- he would be silly to use a foreach loop (to stick with our example) there...
Btw. If you look at Microsofts latest project examples for the new platforms (WP8, Metro ...) about performance/memory heavy tasks (Imaging...) you will see the same approach - one project for RAD designing the GUI in .NET, and another one for the performance/memory optimized code written in NATIVE C++ (not CLR).
So in the end I think you are right, what separates the "pros from the amateurs" is the knowledge about such little things like for vs. foreach, break, string-allocation,... and even more in a complex "framework" like .NET with various programming languages and "flavours". This makes the sometimes little but crucial difference in projects like the one you descriped (I imagine you smiling after they messed it up because not following your recommendations ).
But it happens often to me that readabillity and "pattern awareness" of code does matter more than "pure" perfomance, because code will run on hardware with more memory than StartTrek authors thought is "much" in the 90's...
Kind regards Johannes
|
|
|
|
|
the first because it will terminate the loop on the first found string, and is thus faster. The second loops the whole array regardless.
If your actions inspire others to dream more, learn more, do more and become more, you are a leader.-John Q. Adams You must accept one of two basic premises: Either we are alone in the universe, or we are not alone in the universe. And either way, the implications are staggering.-Wernher von Braun Only two things are infinite, the universe and human stupidity, and I'm not sure about the former.-Albert Einstein
|
|
|
|
|
I know you were asking for another thing - but the code and alternative you presented are not equal - so someone has to choose Option 1, because this code is the correct one (exit Loop on found string) - so you second DoSomething should do break the loop after a value was found (you will have to add the brackets you left away ) Btw. are you new on "the Internet"? It feels about the billionth discussion about early exit vs. single return point - AFAIK this will forever be a matter of style and choise...
|
|
|
|
|
How about with little refinements?
bool DoSomething(string[] values)
{
bool blnReturnValue = false;
foreach (string s in values)
{
blnReturnValue = s.Equals("ABC");
if (blnReturnValue)
break;
}
return (blnReturnValue);
}
Vasudevan Deepak Kumar
Personal Homepage
BRAINWAVE/1.0
Status-Code: 404
Status-Text: The requested brain could not be found. It may have been deleted or never installed. --Brisingr Aerowing
|
|
|
|
|
|
I haven't touched C++ for donkey's years, but this sounds like something that could be fixed with a #pragma pack[^].
"These people looked deep within my soul and assigned me a number based on the order in which I joined."
- Homer
|
|
|
|
|
Your donkey's ears have done you no harm. It could be fixed with a #pragma pack if I wasn't writing code to be portable across compilers
I've moved the struct s outside the class and given them static member functions to report their own size. These only get built into one Dll in one place so they can only return one set of values.
Still a way to got to clean up this mess but at least I've got a handle or what's going on now I think.
"The secret of happiness is freedom, and the secret of freedom, courage."
Thucydides (B.C. 460-400)
|
|
|
|
|