Bad Coding Standards

Just in case you didn't have enough bad CodingStandards in your organization, ;-> here are a few additional suggestions:

One of the traps of BadCodingStandards, when paired with ineffectively led code reviews, is that they give non-productive developers a tool for obstructing productive developers.

Funny how the same deficient beliefs melt away when the developers in question are programming together trying to pass a UnitTest.

Maybe, maybe not. If the "productive" developer merely appears productive because he can sling thousands of lines of bad code in a day, he will end up costing you more in the long run. The "unproductive" developer who spots the defects in the code may just be saving your bacon.

And now for something completely different! Not a 'bad standard' for coding, but standard for 'bad coding', especially in Java. Serious advice for those who want JobSecurity from obfuscated code: HowToWriteUnmaintainableCode, by RoedyGreen.

-- DickBotting

Hrm... Points 22 and 47 remind me of the X window system.

I'm gonna try really, really hard not to rub too many people's fur the wrong way here. Coding standards - no matter how bad - are a Good Thing from where I sit. At least the client has thought about it a little bit. Even if one of their pimply-faced, aged 23, fresh grads dragged something off of an Internet page and thrashed it into a three page document that's better than nothing at all. With a coding standard you have something to point to and say, "See? It's right there." I will code to anybody's coding standard no matter how bizarre or obtuse. Having even a stupid one eliminates a lot of "religious wars" before they get any steam whatsoever. At my current client I have had to point out a lot of the flaws in the existing standard, of course. The doc gets revised whenever holes show up. Simple. -- MartySchrader 

 #define TRY_START try {
 #define TRY_END }


 void foo() {
TRY_START
 bar();
TRY_END
...
 }
Unfortunately, I'm not making this up...

Nor was the person who originally coded that. It is a hack, but it isn't necessarily an unwarranted hack - you can simulate exceptions using macros for the (lame) compilers that don't support them. Personally, I just say let old junky compilers die without honor, but if you're developing some uberportable code you have to deal with the ugly issues like the above.

Even more unfortunately, this was not the case. TRY_START and TRY_END have been used because they're so much easier to see that a lower case word a couple of braces.

(MicroSoft's 16-bit Visual "C++" compiler had no exceptions. How they could call it C++ when it lacked such basic functionality is beyond me, but it did.)

They could, because exceptions were simply not part of C++ in the beginning. And a lot of programmers can happily live without them.

I'm sure you're not [making up the try/catch macros] because I've seen worse. As I recall, the GEM developer's kit (circa mid-80's) had a file called portab.h that went something like this:

#define begin {

#define end }

How anyone could possibly think that by making their code look vaguely like Pascal makes it more portable is beyond me. -- AlexValdez

There was a similar pack of definitions in the source code for the Bourne shell I once looked at... made C look just like Pascal. -- DickBotting

[As a matter of fact, you can thank Steve Bourne himself for that. He was an Algol fan and did not care for C braces and such. We made fun of him for that breach of good sense (of redefining the language with macros, not for preferring Algol) even back in the 1970s.]

I once programmed C at a company where I found, in an .h file, this bit of egregious macro definition: 

 #define TRUE 1
 #define FALSE 0
... and I thought that was bad enough, until a week later, in another .h file, I came across: 
 #define TRUE 0
 #define FALSE 1

I have seen this

in begin.h {

in end.h }

in main.cpp

#include "begin.h" 

 .... do some thing
#include "end.h"

Way back in 1969, after I had learned PL/1 and then moved from OS/360 to DOS (which had a rather less-capable compiler), I worked in an organization where [for efficiency reasons] they insisted that rather than use procedure calls you had to set variables, set a return label into a label variable, and then GOTO the subroutine (which would return using a GOTO to its global return label variable). Like: 

param1 = "hello"
param2 = "world"
p1ret = next
goto proc1
next: 
/* continue program */
I don't think they had much idea of the importance of maintainability on that project.

They probably had scars from previous environments. And maybe the procedure calls were worse than we would imagine. I mis-read your comment at first and it jogged some memories and I found this: "The original IBM Algol compiler did a GETMAIN system call on every procedure call" on http://compilers.iecc.com/comparch/article/96-07-031 ...which discusses why programmer time wasn't considered valuable compared with efficiency.

Each function gets a banner generated with FigLet.

Is this real?

Sorta. A co-worker came in and asked me where banner(1) was. I suggested figlet instead, installed it, and then asked why. He wanted it for his header comments. (Fortunately, he doesn't set project-wide policy.) 

 //_    __  __ ____  ____   ____ _            _
 //    / \  |  \/  |  _ \/ ___| / ___| |_ __ ___ __| |
 //   / _ \ | |\/| | |_) \___ \| |   | '_ ` _ \ / _` |
 //  / ___ \| |  | |  __/ ___) | |___| | | | | | (_| |
 // /_/   \_\_|  |_|_|   |____/\____ |_| |_| |_|\__,_|
 //
 //
 //  Do enough parsing to determine handler for the command and then pass data
 //  etc. to the handler
My boss requires this. Fortunately, he doesn't get too upset when I "forget".

Try this: It takes about 2 seconds longer to scroll to the code you are looking for. If you average looking at 300 routines a day (including re-visits to the same one), then you spend 2 x 300 = 600 seconds or about 10 minutes a day scrolling past them. That's about 0.17 hours a day. If a year averages about 260 work days, then those banners require a total of about 44 hours a year (0.17 x 260). If your salary plus company overhead is about $60/hr, then those banners cost your company about $2640 per year (60 x 44). Present that cost analysis to your boss, and see what he says. Ask him if the marketing value of the banners makes up for that amount. Please report back to this wiki his/her reaction.

I had a co-worker complain about how my code did not conform to the "standard" 80 characters per line because their editor (Emacs) was setup to wrap text at 80 characters. I write code on dual flat 20" panel screens both running at 1600 x 1200. I can see well over 80 character per line easily. A lots of the code I write is significantly more legible at greater then 80 characters. I don't have to purposely create variable names x,y,z, x1,y1,z2, etc just so I can do computations in a small space. I can name these variable: centerPointX, centerPointY, rotatingPointX, rotatingPointY, etc.. Is there anyone else out there who agrees? And for you die hard 80 line people, why can't you get a bigger screen. Seeing more code will make you more productive. Arggg... -- AnonymousDonor

I DO have a bigger screen. That's so I can see my 80-column Emacs window PLUS another window (e.g., a browser displaying JavaDoc pages).

I don't stick to a hard & fast 80 chars, but I think it's good to keep it under about 90. This has more to do with TenWordLine than legacy terminal displays. Your eyes have to read sideways if the lines get too long, rather than skimming downwards.

I do tend to break this a lot when the line isn't all that important, for example unit tests and required parameters where there presence is required by the method name. I never read those anyway, so I don't need to worry too much about readability.

Why not break the line vertically, eg. after assignment statements or operators, instead of using short variable names? -- JonathanTang

The 80 character rule is particularly pointless. Based on monitors, eyesight, and screen space taken up by IDEs, one may be able to type fewer or more than 80 characters on a line. Go ahead and type in code that best fits the editior environment currently in use. I usually do not worry about a couple of characters that go past the end of the screen and only fix the format if I find it too annoying. Don't try to predict what some unknown person may use in the future to read and edit code. Finally, get on board with the rest of the team and using a common editor and configuration. Don't expect everyone else to do handstands to allow you to use your "clearly superior" code editor. --WayneMack

FWIW, I stick to a 99 character right column in all my code. Why? So I can print it out without having anything wrap onto the next line. (I print using Courier New@9pt on A4 paper with line numbers so I can do code reviews of my own stuff on the train!) -- BevanArps.

What is the advantage of having everyone on the team use the same editor, that outweighs the advantage of everyone on the team using an editor they find comfortable and productive? -- GarethMcCaughan

If you're writing open-source software that can and will be hacked on by anyone who wants to bother, it might be polite to not assume everyone in the universe has a 1600x1200 screen. Besides, life can take one weird places, and it might be nice to have code one can comfortably mess with while SSH'd into your server from a VT220 terminal (true situation). At least, that's my excuse for sticking to more-or-less 80 columns. -- SimonHeath

Horizontal scrolling can make code very hard to read. (Unless your language is optimized for horizontally written content.) Unless everyone agrees to the same limit, some will be left horizontal scrolling or everyone will have to adopt to identical environments.

I've found having a horizontal limit helps to guide me to avoid cramming multiple concepts into one line. Compare 

process_parts(numWidgets + numGadgets, isInitialized && isIdle, context->get_rules_for(PROCESSING));
To 
numParts = numWidgets + numGadgets;
isReady = isInitialized && isIdle;
processRules = context->get_rules_for(PROCESSING);
process_parts(numParts, isReady, processRules);
Or 
process_parts(
numWidgets + numGadgets,  // Number of parts
isInitialized && isIdle,  // Ready?
context->get_rules_for(PROCESSING)); // Process Rules
Some places strictly restrict lines to 74 characters. No more, or your code will not be allowed.

120 seems to work well for most wide and double-screen environments -- considering multiple windows, side panels and all.

Limiting width to 80 seems to create problems with reasonably named variables and methods, even if nested conditions and loops are kept to a reasonable level (line one or two). Maybe it would work with very highly object-oriented code, but it doesn't seem to work well with most Java I've seen -- at least not without MAJOR rework. -- JeffGrigg

PaulGraham and some other writers like to maintain a total text width of a few inches for scannability. I suspect the same applies to code. The rest of the screen spacec can be used for utilities or for a vertical split and another batch of code. --JesseMillikan

I would be interested in seeing how he structures his Lisp code, because it is not easy to maintain only a few inches when you consider how deeply Lisp code nests. Most Lisp coding conventions I've seen prefer a diagonal layout that can trivially exceed 80 columns, leading to such abhorrences as: 

  ( ....lots of stuff and nesting here; vertical line is right-hand edge of screen ....
    (cond               |
      ((condition-1)    |
       (result-1))      |
      ((condition-2)    |
       (result-2))      |
      (t                |
       result-3)))      |
  )))))...))            |
I look at code like this and cannot help but think that a combination of better factoring and a MUCH wider screen would be absolutely critical to good, legible Lisp coding. --SamuelFalvo?

Gee, you think some side effects here? Maybe? 

 if (a == 5) {
printf("a is equal to 5\n");
b++;
 }
 else {
printf("a is not equal to 5\n");
b--;
 }
Actually, no, I don't see any side effects here, because it is not clear that b is a locally defined and used variable or not. And, a certainly isn't being modified out from the conditional testing. So, no, there is insufficient data to say whether there are any side effects happening here. --SamuelFalvo?

The lifetime and usage of b doesn't matter. The ++ operator usually has the side effect of changing b's value (same with --). Even if these operators don't have side effects, there is still the printf statements. --MartinShobe?

I agree with MartinShobe?. The 'printf' is cause for a side-effect all on its lonesome (or a 'computation-effect' if you don't like calling intentional communications 'side-effects', but 'SideEffect' is the term that has come into wide use). And the statements 'b++' and 'b--' also have a consequence that qualifies for the term 'SideEffect' (having an effect that extends beyond the statement), with a possible exception being if you overloaded those operators for 'b' to mean something functional. This is true even if you immediately discard 'b' upon exiting the procedure in which it is declared, though in that case you might have been able to say (if you also rid yourself of the 'printf') that the procedure (as a whole) had no side-effects.

In general, if you program with statements rather than expressions, there must be SideEffects. After all, a statement without any SideEffects is a statement that can be optimized by excising it entirely from your program.

What what you're saying is correct, the original post did not specify any particular context. Every response to mine so far has established a more specific context. Does ++ and -- perform a side-effect? Sure. At the statement level. But across a whole procedure, the answer may not be so clear.

Statements with no side-effects are still useful. if-statements are side-effect free, unless you use sloppily written macros. All a side-effect is, is something that is not implicit in the inputs and outputs. There is no law that says a side-effect-free computation must do nothing at all.

That being said, I'm still utterly bamboozled why this is here at all; why should side-effects of the form b++/b-- have any bearing on good/bad programming standards, as illustrated above? If the author had written: 

 void someProcedure(int a) {
     if (a == 5) {
         printf("a is equal to 5\n");
         b++;
     } else {
         printf("a is not equal to 5\n");
         b--;
     }
 }
this is obviously a bad thing, since the post-increment/decrement operators are touching stuff that is not explicitly passed into the procedure as a variable. Rewriting this in a side-effect-free form: 

 int someFunction(int a) {
     int aEquals5 = a == 5;
     char *perhaps = aEquals5? " " : " not ";
     printf("a is%sequal to 5\n", perhaps);
     return b + (aEquals5 ? 1 : -1);
 }
This is positively side-effect free, except for the printf(). --SamuelFalvo?

I also haven't a clue as to the context that brought this into BadCodingStandards. I was simply responding to your own statement. As to whether the 'b++' and 'b--' are a 'bad' thing - that is still questionable and not obvious. I've never seen a convincing argument that SideEffects are fundamentally always a 'bad' thing, and the need to hack around communications issues in Haskell, and the prevalence of databases and filesystems, all seem to be points to the contrary.

Programming with side-effects certainly ought to be controllable or at least traceable, such that you can track all non-local changes back to their source, and it would be nice if such could be compiler and programming-environment supported. But if 'b' is well defined with a comment to the effect '//balance of someFunction(x) where x=5', that requirement is minimally met. 

 if (foo) {
   // ...
 } // end if


 void foo() {
   // ...
 } // end foo
We have somebody at work that does this excessively, even for single line blocks. Here are my issues with it:

I can understand the reason for using this kind of comment for very long blocks and where many braces close at the same time, but honestly most IDEs can highlight matching braces/parens for you, which renders this whole thing obsolete.

And if you find the need to this, I'd suggest you look to refactor your code to make it smaller/less nested.

MikeWeller

Many on the list are merely personal preferences. What bothers or slows down one person may not another. Everybody is different. But, it's good to weigh, understand, and document the pro's and con's of each choice so at least one is making an informed decision when they create a set of standards.

Note: Much of this page reeks of PersonalChoiceElevatedToMoralImperative. Watch yersef. Ah, a source of many BadCodingStandards...

See: BadProgrammer

CategoryCodingIssues

EditText of this page (last edited November 12, 2013) or FindPage with title or text search