From the Museum of Ugly Code
-
The only hungarian notation I really like is sticking a little "p" in front of a variable to indicate it's a pointer. Just makes it easier to understand what I am doing. Microsoft loves to go wonkers with it (UINT32, and yes I know that's a type), and I have seen some code (due to coding standards) look like pStrConst_Variablename.
Charlie Gilley <italic>Stuck in a dysfunctional matrix from which I must escape... "Where liberty dwells, there is my country." B. Franklin, 1783 “They who can give up essential liberty to obtain a little temporary safety deserve neither liberty nor safety.” BF, 1759
Personally my favorite is :
LPSTR lpszName // lp = long pointer s = string z = zero (null).
// It's a long pointer to a null terminated string, of course.:rolleyes: Thos were the good old days* when there were no Internet to look that stuff up. All you had was The Petzold and windows.h. :) *Not really all that great for learning to program. :)
-
den2k88 wrote:
scoped variables ... apart from making the code more difficult to read and to maintain.
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.
den2k88 wrote:
always initialize your variables
Indeed. Yes!
den2k88 wrote:
absolutely, positively sure
That works until it doesn't. See first rule: initialize!
#SupportHeForShe Government can give you nothing but what it takes from somebody else. A government big enough to give you everything you want is big enough to take everything you've got, including your freedom.-Ezra Taft Benson You must accept 1 of 2 basic premises: Either we are alone in the universe or we are not alone. Either way, the implications are staggering!-Wernher von Braun
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
-
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());First time I saw something like thins in a C++ code base, I thought "That's clever!" It was only the second thought that I remembered how much I bloody hate maintaining such "clever" solutions. We coders tend to be attracted to logical puzzles. It takes willpower to steer away from something clever in favor of something simple. Either that or enough experience to know how much of a PITA clever code can be.
-
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.....