From the Museum of Ugly Code
-
charlieg wrote:
if (bVariable = someothervar)
That's a very ugly one. I had to try it in JS. It works, always runs. C# at least gives you an error "cannot implicitly convert 'int' to 'bool'
That's nothing. I had this once in JS ` if(functionName) //that's a reference, not a call! { //Do something } else { //Some other equally poor code }` It came up at pull request that as it's a function reference, it will always be true, and thus got rejected. Anyway, the bloke went mental, telling me I couldn't reject it as I hadn't run the code to see if it worked.... He started randomly rejecting my pull requests after that.
-
TheGreatAndPowerfulOz wrote:
I actually find using scoped variables makes code easier to read and maintain. :-D YMMV. And it has fixed and avoided many a bug for me.
If the code is well written, I concur. When they are used in 300-500 lines jack-of-all-trades functions with dozens of scopes inside and badly named variables on the other hand...
TheGreatAndPowerfulOz wrote:
That works until it doesn't. See first rule: initialize!
My experience exactly :D
GCS d-- s-/++ a- C++++ U+++ P- L+@ E-- W++ N+ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t+ 5? X R+++ tv-- b+(+++) DI+++ D++ G e++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X
Scoped vars make so much sense! They stop shitty devs just using those variables and other functions polluting them
-
That's nothing. I had this once in JS ` if(functionName) //that's a reference, not a call! { //Do something } else { //Some other equally poor code }` It came up at pull request that as it's a function reference, it will always be true, and thus got rejected. Anyway, the bloke went mental, telling me I couldn't reject it as I hadn't run the code to see if it worked.... He started randomly rejecting my pull requests after that.
no, it won't always be true. Consider:
var functionName=null;
//
// ...
//
if (functionName) {
alert('true')
} else {
alert('false')
}functionName = function() {
// Some useful stuff
}if (functionName) {
alert('true')
} else {
alert('false')
}There are plenty of use cases where testing a function for existence is perfectly valid. I guess not in that particular case though.
-
while (!sourceSubFolders.atEnd()){
// do something with files in the folder and don't use "continue"
sourceSubFolders.moveNext();
} -
no, it won't always be true. Consider:
var functionName=null;
//
// ...
//
if (functionName) {
alert('true')
} else {
alert('false')
}functionName = function() {
// Some useful stuff
}if (functionName) {
alert('true')
} else {
alert('false')
}There are plenty of use cases where testing a function for existence is perfectly valid. I guess not in that particular case though.
Fair point! In this case, it was just poor
-
Scoped vars make so much sense! They stop shitty devs just using those variables and other functions polluting them
Just how a scoped variable can be polluted by another function? I'm not talking about local variables vs global ones - that would be silly - but of variables declared in nested blocks, as
//code
if (condition){
int xyz;
int abc;
// code
}
// other codeabc and xyz will exist only inside the if block and then disappear again. Is that bad? No, if the code is well written (short clear functions) and those variables are positively needed only in that condition statement. If the code is long and complex jous just end up in a screen of code where there are two "things" named abc and xyz that weren't used anywhere else... it's confusing, to say the least. Also there is no real benefit since all those variables are stack based so no allocation ever takes place - no need to "save those bytes on the stack" or "save the allocation time".
GCS d-- s-/++ a- C++++ U+++ P- L+@ E-- W++ N+ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t+ 5? X R+++ tv-- b+(+++) DI+++ D++ G e++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X
-
Just how a scoped variable can be polluted by another function? I'm not talking about local variables vs global ones - that would be silly - but of variables declared in nested blocks, as
//code
if (condition){
int xyz;
int abc;
// code
}
// other codeabc and xyz will exist only inside the if block and then disappear again. Is that bad? No, if the code is well written (short clear functions) and those variables are positively needed only in that condition statement. If the code is long and complex jous just end up in a screen of code where there are two "things" named abc and xyz that weren't used anywhere else... it's confusing, to say the least. Also there is no real benefit since all those variables are stack based so no allocation ever takes place - no need to "save those bytes on the stack" or "save the allocation time".
GCS d-- s-/++ a- C++++ U+++ P- L+@ E-- W++ N+ o+ K- w+++ O? M-- V? PS+ PE- Y+ PGP t+ 5? X R+++ tv-- b+(+++) DI+++ D++ G e++ h--- ++>+++ y+++* Weapons extension: ma- k++ F+2 X
Oh god, yeh that's ugly.
-
That's nothing. I had this once in JS ` if(functionName) //that's a reference, not a call! { //Do something } else { //Some other equally poor code }` It came up at pull request that as it's a function reference, it will always be true, and thus got rejected. Anyway, the bloke went mental, telling me I couldn't reject it as I hadn't run the code to see if it worked.... He started randomly rejecting my pull requests after that.
-
When you see this, it only takes a few extra seconds to think about what the dev intends. But... X|
for (;!objSourceSubFolders.atEnd(); objSourceSubFolders.moveNext()) {
// do something with the files in the folder.
}First of all, not sure why you need to use Hungarian notation to signal that this is an obj. But, more importantly, does the dev not know of the existence of the while loop? Or did he think this was an innovative approach? :sigh: Look how much simpler this is to read.
while (!sourceSubFolders.atEnd()){
// do something with files in the folder
sourceSubFolders.moveNext();
}EDIT This one is for all you people. You know who you are! :rolleyes:
do {
// do something with files in the folder
sourceSubFolders.moveNext();
}
while (!sourceSubFolders.atEnd());Regarding Hungarian Notation, here is an interesting article: [Making Wrong Code Look Wrong – Joel on Software](https://www.joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong/)
TOMZ_KV
-
Regarding Hungarian Notation, here is an interesting article: [Making Wrong Code Look Wrong – Joel on Software](https://www.joelonsoftware.com/2005/05/11/making-wrong-code-look-wrong/)
TOMZ_KV
That's a very good article. I always like analogies because they help so much. And this :
char* dest, src;
is a great example of code that once you know, you know, but after not seeing it for years kind of makes you pause and think, "wait, is src a char* too or just a char? That's exactly like the weird for loop I was displaying. It just makes you think extra for no reason. And I liked Hungarian for years. I still do, but I'm kind of one the fence about it. You can tell what the types are, even if there is not intellisense. The point of the obj thing was that it wasn't helpful because I can tell you're calling a method on the thing so I can tell it's an object anyways. I prefixes like n and i did help so I didn't have to always back up the code and look to see what the thing was. Then on the other hand, go ahead and name vars so I can tell that it will contain numeric values versus strings, etc.
-
When you see this, it only takes a few extra seconds to think about what the dev intends. But... X|
for (;!objSourceSubFolders.atEnd(); objSourceSubFolders.moveNext()) {
// do something with the files in the folder.
}First of all, not sure why you need to use Hungarian notation to signal that this is an obj. But, more importantly, does the dev not know of the existence of the while loop? Or did he think this was an innovative approach? :sigh: Look how much simpler this is to read.
while (!sourceSubFolders.atEnd()){
// do something with files in the folder
sourceSubFolders.moveNext();
}EDIT This one is for all you people. You know who you are! :rolleyes:
do {
// do something with files in the folder
sourceSubFolders.moveNext();
}
while (!sourceSubFolders.atEnd());You're right - he totally should have used a while loop
while (objSourceSubFolders.moveNext())
{
// do something with the files in the folder.if (objSourceSubFolders.atEnd()) goto Ended;}
Ended:cheers Chris Maunder
-
You're right - he totally should have used a while loop
while (objSourceSubFolders.moveNext())
{
// do something with the files in the folder.if (objSourceSubFolders.atEnd()) goto Ended;}
Ended:cheers Chris Maunder
-
When you see this, it only takes a few extra seconds to think about what the dev intends. But... X|
for (;!objSourceSubFolders.atEnd(); objSourceSubFolders.moveNext()) {
// do something with the files in the folder.
}First of all, not sure why you need to use Hungarian notation to signal that this is an obj. But, more importantly, does the dev not know of the existence of the while loop? Or did he think this was an innovative approach? :sigh: Look how much simpler this is to read.
while (!sourceSubFolders.atEnd()){
// do something with files in the folder
sourceSubFolders.moveNext();
}EDIT This one is for all you people. You know who you are! :rolleyes:
do {
// do something with files in the folder
sourceSubFolders.moveNext();
}
while (!sourceSubFolders.atEnd());I like the first one better. :laugh: I was expecting to see some horrid error-prone mess.
raddevus wrote:
while (!sourceSubFolders.atEnd()){ sourceSubFolders.moveNext(); // do something with files in the folder }
Classic! The proposed rewrite skips the first item. Well, I think we've all been there. ;) Hey, remember this ugly beauty from System.Runtime.Remoting?
static StringBuilder vsb = new StringBuilder();
internal static string IsValidUrl(string value)
{
if (value == null)
{
return "\"\"";
}vsb.Length= 0; vsb.Append("@\\""); for (int i=0; i -
I like the first one better. :laugh: I was expecting to see some horrid error-prone mess.
raddevus wrote:
while (!sourceSubFolders.atEnd()){ sourceSubFolders.moveNext(); // do something with files in the folder }
Classic! The proposed rewrite skips the first item. Well, I think we've all been there. ;) Hey, remember this ugly beauty from System.Runtime.Remoting?
static StringBuilder vsb = new StringBuilder();
internal static string IsValidUrl(string value)
{
if (value == null)
{
return "\"\"";
}vsb.Length= 0; vsb.Append("@\\""); for (int i=0; iBaraaaaaa wrote:
I like the first one better.
There's not accounting for taste. :rolleyes:
Baraaaaaa wrote:
Classic! The proposed rewrite skips the first item.
I know. It was a quick analysis of the original ugly code but I was too lazy to go back and fix it. This is yet more proof that the original ugly code will cause problems for future maintenance devs. :rolleyes:
-
When you see this, it only takes a few extra seconds to think about what the dev intends. But... X|
for (;!objSourceSubFolders.atEnd(); objSourceSubFolders.moveNext()) {
// do something with the files in the folder.
}First of all, not sure why you need to use Hungarian notation to signal that this is an obj. But, more importantly, does the dev not know of the existence of the while loop? Or did he think this was an innovative approach? :sigh: Look how much simpler this is to read.
while (!sourceSubFolders.atEnd()){
// do something with files in the folder
sourceSubFolders.moveNext();
}EDIT This one is for all you people. You know who you are! :rolleyes:
do {
// do something with files in the folder
sourceSubFolders.moveNext();
}
while (!sourceSubFolders.atEnd());It may have been a "fix" for even uglier code. I have to remind myself (sometimes) not to assume what was in the minds of the those that came before. And, if you've been coding "other" for-loops all day ... "highway hypnosis".
"(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal
-
It may have been a "fix" for even uglier code. I have to remind myself (sometimes) not to assume what was in the minds of the those that came before. And, if you've been coding "other" for-loops all day ... "highway hypnosis".
"(I) am amazed to see myself here rather than there ... now rather than then". ― Blaise Pascal
My original post has been a litmus test. What I'm doing is making a list of all the devs (very few) here at CP who actually say, "well, I understand this...life is real and code is ugly at times". These are the people who I will accept their opinions when I write articles and post here to CP. The rest I will ignore. :rolleyes: You are on the list for your understanding. Don't let that get out or the other Engineers will harass you. :laugh:
-
When you see this, it only takes a few extra seconds to think about what the dev intends. But... X|
for (;!objSourceSubFolders.atEnd(); objSourceSubFolders.moveNext()) {
// do something with the files in the folder.
}First of all, not sure why you need to use Hungarian notation to signal that this is an obj. But, more importantly, does the dev not know of the existence of the while loop? Or did he think this was an innovative approach? :sigh: Look how much simpler this is to read.
while (!sourceSubFolders.atEnd()){
// do something with files in the folder
sourceSubFolders.moveNext();
}EDIT This one is for all you people. You know who you are! :rolleyes:
do {
// do something with files in the folder
sourceSubFolders.moveNext();
}
while (!sourceSubFolders.atEnd());Your logic isn't quite the same. The original logic does moveNext() at the end of the loop, always. While your logic does it first, before "doing something". The for loop variant guarantees that moveNext() is done for each loop iteration. The while loop variant can be goofed up by other devs adding a continue before doing the moveNext() -- speaking from experience, having had to fix these sorts of problems.....
-
Your logic isn't quite the same. The original logic does moveNext() at the end of the loop, always. While your logic does it first, before "doing something". The for loop variant guarantees that moveNext() is done for each loop iteration. The while loop variant can be goofed up by other devs adding a continue before doing the moveNext() -- speaking from experience, having had to fix these sorts of problems.....
wrote:
Your logic isn't quite the same.
Yes, others have pointed this out. I just wrote the code sample real fast. Now, the subtle error is bait for people with OCD. :rolleyes: Even the explanation of using the for loop in this case actually stinks the place up, because the original code writer probably didn't use critical thinking to consider that the loop will run once and then only increment to the next file after it gets to the bottom (after the first time through the loop). I could've made it a do...while. :) NOte: I'm not directing this criticism at you, just at any explanation of using the for loop for that reason.
-
When you see this, it only takes a few extra seconds to think about what the dev intends. But... X|
for (;!objSourceSubFolders.atEnd(); objSourceSubFolders.moveNext()) {
// do something with the files in the folder.
}First of all, not sure why you need to use Hungarian notation to signal that this is an obj. But, more importantly, does the dev not know of the existence of the while loop? Or did he think this was an innovative approach? :sigh: Look how much simpler this is to read.
while (!sourceSubFolders.atEnd()){
// do something with files in the folder
sourceSubFolders.moveNext();
}EDIT This one is for all you people. You know who you are! :rolleyes:
do {
// do something with files in the folder
sourceSubFolders.moveNext();
}
while (!sourceSubFolders.atEnd());This goes back to the (bad) old C days. To be cool, one would use fewer statements. For example, a forever loop we could use:
for (;;)
{
// Do something
}or
while (1)
{
// Do something
}It became in style to use the for. Besides, back in the days, C was done in Unix and code wasn't meant to be read by humans. And, the compiler didn't care how you formed your code.