![]() |
|
The popularity of ugly code |
Ariesnl
Member #2,902
November 2002
![]() |
I see a lot of people writing code in a style my former teacher would kick my butt for... 1
2for(;;)
3{
4 if (blCondition == true) break;
5}
6
7
8for(;blFound == true;)
9{
10 if(seeksomething() == success)
11 {
12 blFound = true
13 }
14
15}
16
17
18
19while(1)
20{
21 switch(something)
22 {
23 case 1:
24 foobar1();
25
26
27 case 2:
28 foobar2();
29
30 default:
31 break;
32 }
33
34}
Why ? is unreadable code the new standard ? is someone affraid a beginner sees what's going on or what ?? these are simplified examples but i've seen for loops that took 5 minutes to decipher, like writing the wole stinky mess in the loop check section.. and_this_Function(){ is ugly
Perhaps one day we will find that the human factor is more complicated than space and time (Jean luc Picard) |
OICW
Member #4,069
November 2003
![]() |
It can just that the old habits die hard. But I feel for you. One thing you could probably do would be to show the authors a book by Steve McConnel - Code complete. And if they do not improve their coding standards, well then beat them with that book until they do [My website][CppReference][Pixelate][Allegators worldwide][Who's online] |
Vanneto
Member #8,643
May 2007
|
Just because people can program doesn't mean they're good at it. The reason you see (mostly) good code on this forum is because people do it as a hobby/it's their passion. They enjoy doing it / learning about it. When you see code outside in the real world its mostly shitty. This is because it was written by people who don't give a fuck. They're in it for the money, much like the majority of people in the majority of professions. Not saying this is wrong or anything, but it is as it is. In capitalist America bank robs you. |
Ariesnl
Member #2,902
November 2002
![]() |
True, Vanetto.. Eventually the whole project collapsed due to some bad bugs custommers got aware of.. Perhaps one day we will find that the human factor is more complicated than space and time (Jean luc Picard) |
jmasterx
Member #11,410
October 2009
|
Another point is that customers who request software do not see the value in quality code. It might take longer to write quality code, but any time updates are needed, the quality code saves a lot of money. But customers do not understand that most of the time. They just want code that works. However, the code in the OP is just stupid. Clearly anyone that writes that kind of code does not understand how programming works or something. They lack the logic necessary to code in the first place so I doubt they could code anything useful. In production code, I often see ugly hacks, but I do not see much code like the OP. Most of that code could be commented out without affecting the result. I guess (hope) that the people at your job writing code like this are not titled programmers. If they are they should be terminated immediately. I understand lazy No F***s coding, but writing code like this yields no benefit. It's longer to write this useless garbage than it is to do it right or even half right. Agui GUI API -> https://github.com/jmasterx/Agui |
Matthew Leverton
Supreme Loser
January 1999
![]() |
I've seen this pattern in languages that support it: switch (true) { case a == 0: break; case a == 1: break; case a == 2: break; }
|
Kris Asick
Member #1,424
July 2001
|
I've always made it a point of making my code visually nice to look at for my own sake, in case I have to take a break from it and come back to it later. Occasionally, this means defying convention. For instance, instead of: for (x = 0; x < 10; x++) { for (y = 0; y < 10; y++) { for (z = 0; z < 10; z++) { // stuff only happens here } } } I'll do the following instead to save on code space and to make it more obvious that I'm looping through a 3-dimensional set of variables: for (x = 0; x < 10; x++) for (y = 0; y < 10; y++) for (z = 0; z < 10; z++) { // stuff only happens here } This makes it more obvious to me that I'm looping through a 3-dimensional set because all three for loops start on the same line, which to me indicates that they're all looping through the same structure and that the results from one loop do not affect the results of the others. Otherwise, I would go with the normal method. Also, when I need to call the same function multiple times in a row with different arguments each time, I'll often add unnecessary spacing between each argument for easier reading, such as in a situation like this: SetValues(0,"The Frist One",100,400,20); SetValues(1,"Number Two",50,200,185); SetValues(2,"I Am Third",2000,0,0); SetValues(3,"This Be the Fourth",250,72,950); SetValues(4,"Always Fifth",0,600,120); I'll usually do this for easier reading and editing: // ID,"Name" , X, Y, Z SetValues(0,"The Frist One" , 100,400, 20); SetValues(1,"Number Two" , 50,200,185); SetValues(2,"I Am Third" ,2000, 0, 0); SetValues(3,"This Be the Fourth", 250, 72,950); SetValues(4,"Always Fifth" , 0,600,120); And yes, I comment my code whenever I do something that's not very clear on its own accord. I think the best way to get people to code in a more appealing way is to simply code that way oneself and see that others follow your example. I will admit, my code used to be VERY ugly when I was still learning how... --- Kris Asick (Gemini) |
Audric
Member #907
January 2001
|
In the example with while(1), I'm not sure your piece of code is the right one because there's no way to leave the loop. I somehow expected some return() statements somewhere. Personally, when I start a while(1) loop, it's because there are multiple exit conditions in different levels of nested ifs, and I just don't see any clearer way to show the logic (typical case is for parsing a file format that includes both variant and repetitive elements) |
l j
Member #10,584
January 2009
![]() |
Sometimes, I write ugly code if I'm trying to quickly test if some of my ideas actually work in practice. Prototypes I make in about or even under an hour. In big projects I however dislike ugly code a lot, to the point I'll just start over again if I messed too many things up, because I can't be bothered cleaning it up. (Might be the reason why I never finish things.) I've also often noticed that people use a switch statement where they could've just used an array, making the code harder than it should be. I also dislike the 'goto is always evil' principle. 1bool endLoop = false;
2for (...condition1...)
3{
4 for (...condition2...)
5 {
6 if (someConditionIsMet)
7 {
8 endLoop = true;
9 break;
10 }
11 }
12 if (endLoop)
13 {
14 break;
15 }
16}
17
18// Vs
19
20for (...condition1...)
21{
22 for (...condition2...)
23 {
24 if (someConditionIsMet)
25 {
26 goto endOfLoop;
27 }
28 }
29}
30endOfLoop:
31
32// Java and C# (and many others)
33// Allow one to label loops, so one can break out of an outer loop inside an inner loop, where goto can be avoided for such a case, however C and C++ do not support this
34
35outerLoop:
36for (...condition1...)
37{
38 for (...condition2...)
39 {
40 if (someConditionIsMet)
41 {
42 break outerLoop;
43 }
44 }
45}
'Smart' uses of the ternary operator. bool isValid() { return (x > 5) ? false : true; // Completely pointless use of the ternary operator } // VS bool isValid() { return x < 5; // Or perhaps // return (x < 5) == true; // Some think it's clearer, I think it's redundant } I don't even know what to say about this. 1
2// Even contains a bug just for added effect
3for (int counter = 0; counter < 2; counter++)
4{
5 switch(counter)
6 {
7 case 0:
8 doA();
9 break;
10 case 1:
11 doB();
12 break;
13 case 2:
14 doC();
15 break;
16}
17
18// Instead of simply doing
19doA();
20doB();
21// doC()?
Before I forget, a teacher at my old school attempted to do this (VB.net), with very little success I might add: myVar & index = 5;
If you don't know, '&' is the string concatenation operator in VB.net That just showcased a complete lack of understanding the very fundamentals of the programming language she was supposed to teach; I do not understand how incompetent people can be in their profession, yet, somehow manage to keep their jobs.
|
J-Gamer
Member #12,491
January 2011
![]() |
taron said: I've also often noticed that people use a switch statement where they could've just used an array, making the code harder than it should be.
Have to admit I have written such switch statements before. Using an array didn't even cross my mind. " There are plenty of wonderful ideas in The Bible, but God isn't one of them." - Derezo |
Neil Roy
Member #2,229
April 2002
![]() |
Yep, I have seen the for(;;) in some code examples in these forums quite a few times and it always bugs me. It just looks ugly. for(;;) { if (blCondition == true) break; } should be.. while(!blCondition) { // do stuff } or do { // do stuff } while(!blCondition) I actually just changed one of my own while(1) loops into a do{}while() loop instead which checks a variable properly in the while() part. It was actually one of the first times I ever used while(1), I had intended it just as a quick hack to test something and almost forgot to properly code it, I don't like code like that normally. --- |
Kris Asick
Member #1,424
July 2001
|
Actually, one thing that bugs me in terms of looking at other people's code is ANYTHING which immediately suggests a potential infinite loop, such as while(1). I've long since learned to avoid coding infinite loops of any kind and I don't think I've EVER coded a while(1) block in my entire life. In fact, most of my loops that are meant to run repeatedly all tend to work like this: bool done = false; do { // stuff goes here // when it's time to exit the loop, set "done" to true } while (!done); Almost every such loop I write that hasn't been finalized also has a check for the Escape key and sets "done" to true when the escape key is pressed. Once finalized, I may have the Escape key bring up a menu or such instead. Also, everyone's mentioned switch() a bit... my coding style for those is slightly different than the norm, again for easier reading: switch (my_var) { case 0: /* If not a lot of code is required, just stick it on a single line */ break; case 1: { // Lots of code required so // use multiple lines // and then break while still indented break; } case 2: { // More stuff // that needs // more lines // and more lines... break; } default: /* Oh good, this one just needs a single line */ }
--- Kris Asick (Gemini) |
Johan Halmén
Member #1,550
September 2001
|
Ariesnl said: Why ? is unreadable code the new standard ? is someone affraid a beginner sees what's going on or what ?? these are simplified examples but i've seen for loops that took 5 minutes to decipher, like writing the wole stinky mess in the loop check section.. Reading that out loud would be something like "Why the question mark is unreadable code, the new standard question mark is someone afraid. A beginner sees what's going on or what question marks..." Ugly code is like ugly written text. It doesn't work. Wait... it does, but the ugly written text doesn't. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Years of thorough research have revealed that what people find beautiful about the Mandelbrot set is not the set itself, but all the rest. |
Neil Roy
Member #2,229
April 2002
![]() |
LMAO Johan, good point. --- |
bamccaig
Member #7,536
July 2006
![]() |
It's not that ugly code is popular, but that incompetence is plentiful. Most people don't even see code on that level. They only see one thing: does it "work". In my experience, they don't even read it to determine that. They run it. Usually once. As long as it seemed to work then they assert that it's fine! You can even point out blatant vulnerabilities in it and they'll deny them at first. -- acc.js | al4anim - Allegro 4 Animation library | Allegro 5 VS/NuGet Guide | Allegro.cc Mockup | Allegro.cc <code> Tag | Allegro 4 Timer Example (w/ Semaphores) | Allegro 5 "Winpkg" (MSVC readme) | Bambot | Blog | C++ STL Container Flowchart | Castopulence Software | Check Return Values | Derail? | Is This A Discussion? Flow Chart | Filesystem Hierarchy Standard | Clean Code Talks - Global State and Singletons | How To Use Header Files | GNU/Linux (Debian, Fedora, Gentoo) | rot (rot13, rot47, rotN) | Streaming |
Ariesnl
Member #2,902
November 2002
![]() |
wow , I see I threw the cludgel into the hennery @Taron 1
2// here you don't know howmany iterations you need at the beginning of the loop
3for (...condition1...)
4{
5 for (...condition2...)
6 {
7 if (someConditionIsMet)
8 {
9 goto endOfLoop;
10 }
11 }
12}
13endOfLoop:
14
15
16// So according to me WHILE is the way to go
17While ((!condition1)||(!condition2))
18{
19 CheckConditions();
20}
@ Johan Halmén.. Perhaps one day we will find that the human factor is more complicated than space and time (Jean luc Picard) |
Yodhe23
Member #8,726
June 2007
|
Kris Asick - www.justanotherturn.com |
pkrcel
Member #14,001
February 2012
|
I believe with no actual proof of concept that the archetypal for( ;; ) and while(1) were SO COMMON back in the days since whoever was coming from assembly and translated their common practices to mesozoic C didn't give a shuck to readability. It is unlikely that Google shares your distaste for capitalism. - Derezo |
Arthur Kalliokoski
Second in Command
February 2005
![]() |
They're just habits. Mathematical notation has lots of weird things in it that could be improved too, but it's "carved in stone" by now, and if you won't learn the idiosyncrasies, you're deemed an idiot. Same for the English language. They all watch too much MSNBC... they get ideas. |
Peter Wang
Member #23
April 2000
|
There's ugly, but for (;;) and while (1) ain't it.
|
Slartibartfast
Member #8,789
June 2007
![]() |
Yodhe23 said: I have noticed that as I have programmed more and more, I have shifted from a very clean, easily readable with lots of spaces, indents and formatting, etc to trying to cram as much concisely onto a single viewable line as possible. Ideally I would like to get the point where no function is longer than a page/screen of text, but that isn't always feasible or entirely convenient.
I use the readable, lots of spaces, indents and formatting style. To compensate, my second monitor is rotated 90 degrees, so I have 1920 pixels of vertical space for code ---- |
Karadoc ~~
Member #2,749
September 2002
![]() |
I just did a search for while(true) in the thing I'm working on, and thankfully I didn't find anything. But I wouldn't put it past me to sometimes use that. Sometimes I find that code is easier to read if the usual conventions aren't followed... For example, I've recent made a bunch of changes like this: 1// old version
2for (...)
3{
4 if (cond1)
5 {
6 if (cond2)
7 {
8 if (cond3)
9 {
10 // do a heap of stuff
11 }
12 }
13 }
14}
15// new version
16for (...)
17{
18 if (!cond1)
19 continue;
20 if (!cond2)
21 continue;
22 if (!cond3)
23 continue;
24 // do a heap of stuff
25}
Usually I don't like to use continue. But I found those those massive if blocks made it hard for me to easily tell which bits of the code were under which condition - and so I figured I'd make it much clearer to myself that all the code is under all of the conditions by getting rid of all the indentation and braces and everything. I guess my point is that I think different styles are ugly at different times. Johan Halmén said: Reading that out loud would be something like "Why the question mark is unreadable code, the new standard question mark is someone afraid. A beginner sees what's going on or what question marks..." Ugly code is like ugly written text. It doesn't work. Wait... it does, but the ugly written text doesn't.
I enjoyed this. ----------- |
Johan Halmén
Member #1,550
September 2001
|
Only yesterday I wrote while(true); The code went something like this: string pop_a_word(void) { string hlp = ""; char letter; do { letter= pop_a_letter(); if (letter == ' ') return hlp; hlp = hlp + letter; } while (true); } I do know how to rewrite it, but is it worth it? ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Years of thorough research have revealed that what people find beautiful about the Mandelbrot set is not the set itself, but all the rest. |
Ariesnl
Member #2,902
November 2002
![]() |
How about this ? 1string pop_a_word(void)
2{
3 string hlp = "";
4 char letter = pop_a_letter();
5
6 while(letter != ' ')
7 {
8 hlp = hlp + letter;
9 letter = pop_a_letter();
10 }
11
12 return hlp;
13}
even better... 1string pop_a_word(void)
2{
3 string hlp = "";
4 char letter = pop_a_letter();
5
6 while((letter != ' ')||(letter != '\0'))
7 {
8 hlp = hlp + letter;
9 letter = pop_a_letter();
10 }
11
12 return hlp;
13}
Perhaps one day we will find that the human factor is more complicated than space and time (Jean luc Picard) |
Johan Halmén
Member #1,550
September 2001
|
Yes. That's how I would have done it. But the question is, is my example with while(true) uglier than those examples? To me, it's kind of ugly to have to write letter = pop_a_letter(); once outside the while block and once inside it. Next thing I might want to do is add something after the pop_a_letter() call, something I don't want to have inside the pop_a_letter() function. But not only that. My example resembles very well how I think the task is performed. It resembles some pseudo-code-like structure I have. "Pop characters one by one and add them to the string, unless it's a space, in which case you return the string". ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Years of thorough research have revealed that what people find beautiful about the Mandelbrot set is not the set itself, but all the rest. |
|
|