last day (15 days later) » 

12:18
6
Q: Random alphanumeric password generator with GOTOs

Sam PearsonI was inspired after this thread on Stack Overflow to create a random 8 character alphanumeric password generator. Sadly, it is closed, so I cannot provide an answer there. Anyway, here is the code. Let me know if there are any bugs/bias going on. using System.Collections.Generic; using System....

Is there a particular reason you're using goto? In this example it's pretty obvious what it's doing, but I've seen some legacy code that was a crippling headache to figure out what a goto was doing when a simple function would have been as clear as glass.
Is using goto an essential motivator for this code? If not, then it probably doesn't warrant a mention in the title.
TKK
TKK
Perhaps I'm assuming too much, but I'm assuming you're a student or novice programmer. Who taught you to use goto? You might want to regard anything else they teach you with suspicion. goto was already considered harmful 25 years ago.
Started programming in '08 - didn't know a thing until '16. I use goto often now to increase branch prediction by avoiding else. I don't agree with the mass consciousness that goto is a code smell. Think for yourself, question authority -- Leary had a point. If you'd like to see what code I've been writing lately, here is a class library I've been working on that powers all of my current apps: github.com/spearson/xofz.Core98
At any company I've worked at, a goto would instantly fail a code review.
12:18
Why aren't you using Char.IsDigit() and Char.IsLetter() instead of your own range tests?
@SamPearson: are you sure about that? In release mode, your goto code produces the same IL bytecode as the combined if statement from 200_success. It's the if that introduces a conditional jump, you get the else for free. goto doesn't change that. The main performance problem here is caused by throwing away more than 75% of the RNG's output. With a bit of math you can get that down to about 3% loss, without introducing bias - resulting in a 4x speedup.
@SamPearson Your reasoning about the performance of goto is 100% off base. But you have a point: most of the answers merely posit that you shouldn’t use goto without showing why the alternatives are superior. — Nevertheless, make no mistake: your use of goto here has no advantage over an if statement, and it has the tangible disadvantage of making your code massively more complex (longer, and less easy to reason about; i.e. to verify its logic, and to debug it).
@SamPearson not to mention that if you consistently use goto you're also (probably) preventing some optimizations compiler might do. Depending where in the compiler pipeline the optimization is applied (near the frontend where goto and if/else differ, or near the backend where they already generated the exact same code) a compiler is probably not optimizing goto (because it's uncommon to be worth of the time and when you're using it then you know what you're doing). I'm not saying that there are not use-cases (there are) but hey're so rare that [cont]
@Barmar: In this case Char.IsDigit() and Char.IsLetter() are probably the wrong choice, since the aim is to create a password with characters from a known range.
[cont] that you may live without writing a goto for most of your career (with few more chances for its little brother goto case). In this case, if performance are not a MEASURED issue (read: you're generating 1,000,000 passwords) then I'd definitely pick the most readable version. It might even be to use GetBytes() to generate a huge buffer and filter out what you do not want: rng.GetBytes(buffer); string password = new String(buffer.Cast<char>().Where(Char.IsLetterOrDigit).Take(‌​length).ToArray())‌​;.
Of course it's slightly more complex because you may need to concatenate mulitple calls but its should be trivial. Given that you need 8 characters and time spent to generate 256 random bytes is negligible...you may even optimize your code for the best case (just use StringBuilder to store chars and put above code in a while (stringBuffer.length < length) (change Take() to actually take only required number of characters...)
12:18
@Bramar I believe Char.IsDigit() and Char.IsLetter() include unicode characters if I'm not mistaken, so those might have unintended consequences for the OP. For Example IsDigit can include: fileformat.info/info/unicode/category/Nd/list.htm
I'm looking at the gotos and can't stop thinking: W.T.F. Either Sam Pearson is deliberately trolling us, or we need to stage an intervention.
@Shelby - Of course they include Unicode characters (0, for example). In this case, we seem to be selecting from the Latin-1 set, given that buffer contains a byte. We could mask that down to ASCII using & 0x7F (and increase the number of random values we actually use, rather than discard).
@SamPearson I also took a look at your repo and I'm not thrilled about what I've seen. You're doing yeald break; in a Select when either the source or the selector is null! This and all the gotos are frightening :-|
@Barmar What's quite funny about your link is that the quote "Premature optimization is the root of all evil" was actually taken from a paper Knuth wrote about how goto should not be considered harmful as a blanket statement.
12:18
@apnorton Interesting observation. I just glanced at the text surrounding the quote, and what he basically is saying is that once you profile the code and determine where the hot spots are, you can use non-structured code to improve it. So if our OP had actually determined that branch prediction failures were killing his performance, he could be excused for using goto.
IMHO, goto is not bad per se. People claiming that do misinterpret the original intent. But it is definitely a code smell and one should be careful where and when to use it. Especially, it shouldn't automatically cause a code rejection, but it should be well justified. In C++ even more than in C, where it is widely used for cleanup in error conditions.
 
3 hours later…
14:59
it's about time to say it again... goto is bad - but not always - sometimes it's good - but I don't know when - maybe when it's not used?

last day (15 days later) »