This question exists because it has historical significance, but it is not considered a good, on-topic question for this site, so please do not use it as evidence that you can ask similar questions here. This question and its answers are frozen and cannot be changed. More info: help center.

closed as not constructive by Mark Trapp Sep 26 '11 at 8:19

As it currently stands, this question is not a good fit for our Q&A format. We expect answers to be supported by facts, references, or expertise, but this question will likely solicit debate, arguments, polling, or extended discussion. If you feel that this question can be improved and possibly reopened, visit the help center for guidance.
If this question can be reworded to fit the rules in the help center, please edit the question.

The real problem with coding standards is the time and effort wasted arguing over whether they are correct or not. Nothing beats a good curly-brace war for creating internecine strife...
–
MZBOct 14 '10 at 16:07

I find (at least now, earlier at school that seemed strange) that Commenting altogether is a bad practice
–
Shady M. NajibSep 9 '10 at 15:52

14

Not only that but I've found that the overhead associated with creating a new class file when you have to put a load of boilerplate at the top actually dissuades devs from creating new classes and encourages enormous unwieldy classes and hence bad design.
–
GazSep 9 '10 at 16:50

13

Disagree! We don't add useless ore reduntand information, but an actual textual explanation of what the function does (in the .h file) and it is so useful! Of course we are committed to maintain it in sync with the code.
–
WizardSep 9 '10 at 18:09

5

@Shady M Najib bad always or bad when allowed to go uncontrolled/unmaintained? Generally, good code will make its purpose obvious enough to avoid the need for comments- but that isn't always the case and I feel that in these scenarios commenting is crucial. I can't think of one bad reason to include XMLDoc comments.
–
Nathan TaylorSep 9 '10 at 23:27

7

A block explaining what it does is good. A block simply re-iterating the types and names of the arguments and return value is bad. When I had to work with a standard mandating the latter, I wrote an emacs script to auto-generate the majority of it.
–
AShellySep 10 '10 at 16:53

The example you have above is clear, but what if a student copies some function call from another app or a book or what ever, and doesn't really understand it? How will the prof know? This stupid rule doesn't allow any grey area (which in his defense, has probably been abused before). That's how I interpret this. ... mind you if I saw this in a non-academic environment it might scare me a bit. ;-)
–
John MacIntyreSep 8 '10 at 19:55

Our company (C#) coding standard called for extensive use of #REGIONs (for those who don't know, it marks blocks of source code that will be collapsted to a single line in Visual Studio). As a consequence, you always opened what seemed to be a nicely structured class, only to find piles and piles of garbage swept under deeply nested rugs of #REGION constructs. You'd even have regions around single lines, e.g. having to fold out a fold out a LOG region to find one single declaration of the Logger. Of course, plenty of methods added after some region was made, were placed in the "wrong" region scope as well. The horror. The horror.

Regions are one of the worst features ever added to Visual Studio; it encourages structuring the surface rather than the actual OO structure.

If you think you need #REGION, I think you need to refactor.
–
Jay BazuziSep 9 '10 at 19:56

23

I generally organize code by regions into constructors, properties, events, methods, and members. It makes managing and navigating the source a cinch (especially in some static utility classes that can grow pretty large). I wouldn't use them any more than that though.
–
Evan PlaiceSep 11 '10 at 8:23

26

We have a simple coding standard: never nest regions. Just use them to group related methods (initialisation, serialisation, properties, etc)
–
Jason WilliamsSep 11 '10 at 21:53

6

The only good purpose of #regions is to hide code that doesn't need to be edited. That could be generated code, or code with a difficult-to-get-right algorithm that you'd rather people not touch, or maybe ugly debugging-code blocks. But not code that people will be working on. For those stuck in a #region shop, I have a macro that collapses to definitions but expands regions. See here: stackoverflow.com/questions/523220/awesome-visual-studio-macros/…
–
KyralessaSep 29 '10 at 1:28

Sorry, but what happens when you refactor the database or when you decide that the length of the VARCHAR needs to be different. Suddenly, you've got to go through all your code and change... oh god. That looks horrific.
–
Tom MorrisSep 8 '10 at 23:32

More seniable: if you need a comment to say what the start of the block was, then the block is too long, or its content is too complex => refactor.
–
RichardSep 9 '10 at 19:56

5

I want to vote down, because long, deeply nested blocks can be hard to sort out, and these comments might help. I want to vote up, because those comments will become wrong (and very confusing) pretty soon, and because long, deeply nested blocks are a sign you need to refactor, not add more comments.
–
Jay BazuziSep 9 '10 at 20:06

7

that was a great idea for a world with no powerful IDE.
–
IAdapterSep 9 '10 at 22:17

9

@Jay in any decent IDE you can highlight one brace and it'll highlight the other corresponding brace. I personally loathe when people do this.
–
Evan PlaiceSep 11 '10 at 8:32

3

While your example is a bit on the crazy side (as they aren't long enough that it would matter and would slow you down when changing the logic), this isn't always a bad thing. Comments like that are really useful for closing namespaces/endif declarations that span an entire file.
–
jsternbergOct 31 '10 at 19:55

@AndrejaKo: this predated <iso646.h>; this was an attempt to make C look like FORTRAN.
–
Niall C.Sep 9 '10 at 2:00

2

Was this really a coding standard? i.e. was there a company policy against writing the operators directly?
–
finnwSep 9 '10 at 9:13

21

Did it also have #define BEGIN { and #DEFINE END } ?
–
JBRWilkinsonSep 9 '10 at 9:37

3

That reminds me of a Daily WTF article I saw that had some C++ programmer have a ton of defines to make it look like Visual Basic (or maybe just Basic, some dialect). #define void Sub, #define } End, things like that.
–
Wayne MApr 13 '11 at 18:28

“Word spaces should not be taken for granted. Ancient Greek, the first alphabet to feature vowels, could be deciphered without word spaces if you sounded it out, and did without them. […] Latin, too, ceased to separate words by the second century. The loss is puzzling, because the eye has to work much harder to read unseparated text. But as the paleographer Paul Saenger has explained, the ancient world did not desire ‘to make reading easier and swifter.’”

@configurator: When the Visual Studio debugger team was working on a feature to let you see the currently-in-flight exception in the watch window, they were going to add a pseudo-variable called "$ex". We didn't notice for a long time. Then we renamed to "$exception", but I still read it as having an 's'.
–
Jay BazuziSep 29 '10 at 6:33

Were originally included for good reasons but were kept long after the original concern became irrelevant? CHECK. The manager was a developer for a database server 1000 years ago and put this coding standard in.

Were in a list so long that it was impossible to remember them all? CHECK. This included 'as much logic should be stored in the database as possible'.

Made you think the author was just trying to leave their mark rather than encouraging good coding practice? CHECK. Keeps coming back to the manager being an ex-database server developer.

I was asked by the software leader of a company to do "simple, rendundant code".
It was forbidden, for example, to add a new parameter to an existing function. You instead had to duplicate the function, leaving the original untouched in order to avoid regressions. No formal testing of course (waste of time).

We were also forbidden from using merge software; each file could only be modified by one programmer at a time. Revision control software was science fiction, of course.

The happiest day of my life was when he was fired (consider that it is very, very difficult to fire someone in Italy).

The only time I've ever heard of anyone not using the STL because it wasn't fast enough, and being right, was for games. EA made their own implementation of the STL for their games. I highly doubt it matters anymore (modern games are GPU limited) and I doubt they use it. But even still, it was an implementation of the STL, not a whole new library. You were still using the STL when using EASTL.
–
Matt OlenikSep 13 '10 at 23:18

1

@Matt: to add to this, EA complaint was centered on memory usage and initialization. Their own implementation consumed less memory, released it sooner, and avoided initializing objects that would be initialized later.
–
Matthieu M.Oct 14 '10 at 18:00

The biggest problem with this sample is the meaningless variable names. Strip away the Hungarian prefixes and some of them are 1 or even 0 characters long.
–
finnwSep 9 '10 at 18:54

8

This is Systems hungarian, which is useful in weakly-typed languages (as it encodes the type information that is critical for working in these languages in the names) - it's useless in strongly typed languages. The better alternative for strongly typed languages is Apps Hungarian, which encodes important information about the usage of a variable (member, pointer, volatile, indexer) - something the language itself provides no support for.
–
Jason WilliamsSep 11 '10 at 21:51

5

Oh please. I've never ever confused a local with a member, and I don't use that silly Hungarian for members/locals/fields/whatever. I think they might be useful for differentiating between different kinds of strings, like 'safe' and 'unsafe', like Joel's example in Making Wrong Code Look Wrong
–
configuratorSep 29 '10 at 3:59

3

@configurator: Joel's example is horrid, he'd be better off having different types, then the compiler would enforce the use.
–
Matthieu M.Oct 14 '10 at 17:57

Do not fix or document any bugs of issues you
find in your own code. The customer
will pay us to identify and fix them
over the next few years.

This wasn't for consumer software, but custom for a single large organization. Needless to say, the customer did pay for years afterwards. May seem trivial, but trying to ignore bugs is harder than finding them.

It led to some pretty cluttered code, especially since the end result was people either just hitting /// to create an empty comment stub for everything or installing GhostDoc and having it add auto-generated comments:

[Edit] The reason I mention this as a ridiculous standard isn't because I think method comments are stupid but because the quality of these comments wasn't enforced in any way and resulted in just creating lots and lots of clutter in the code files. There are better ways of creating meaningful code docs than blind "must have a comment" build requirement.

+1 Ugh I hate this. I think if your using software to generate comments then you dont need them.
–
bleevoSep 8 '10 at 23:40

9

I don't think this is a bad rule. When reading a method that I have to maintain for the first time it helps a lot if I have specifications for all the arguments. There are usualy subtleties (e.g. what happens if the argument is null, what if it's an empty collection, the name of a nonexistent file etc.) Another good (IMHO) rule is method names should be verbs but in your example it's a noun.
–
finnwSep 9 '10 at 9:18

3

@finnw I think it's a good practice, but a bad standard. If developers are on board and writing meaningful method comments when they're warranted (exception details, etc), that's great. If not, you end up with a big mess. And in the former case, you don't need compilation-level enforcement at all.
–
Anna Lear♦Sep 9 '10 at 13:27

7

Classic case of undocumentation. Comments that don't tell anything apart from the blatantly obvious should be killed on sight.
–
CumbayahSep 9 '10 at 17:05

Those who started the policy were long gone and those that followed kept it going. I started within the same week as the new CTO (friend of mine) and we both looked at this and said WTF?
–
Jim AMar 30 '11 at 11:06

Some of the places I've worked with insisted on commenting out unused or deprecated code instead of deleting it. Instead of trusting the VCS for history, etc. it was painfully maintained in the files through commented out code.

The big problem I found with this is that you often had no idea why the code was commented out. Was it because some dev was actively making changes and wanted to keep it around for reference or was it no longer needed?

The worst coding standard I've ever participated in is code bases which had none at all. I'd rather follow a coding standard I completely disagree with than work in code bases where there is none at all. It makes it that much harder to learn new parts of the code base.

I had a job years ago where all our code had to be left-aligned - no indenting. The guy who came up with that policy disliked having to scroll back and forth horizontally when viewing long lines of code, equating it playing ping-pong with his eyes.

If you need to scroll horizontally (for example more than half a page) there's probably something wrong as well. No indenting isn't good either as it makes code completely unreadable. I try to stick with a 78-col limit, but if required to go over that amount I don't mind, but I do try to avoid it.
–
HtbaaMar 16 '11 at 15:11

A contractor working at a large bank insisted that following the standards were the best ever. The application was written in dBase/Clipper which he was the sole developer for and of course he came up with the standard.

Everything is in upper case. I mean everything, including the rare comments he made.

No indentation.

Variable naming was something along the lines of APRGNAME. A = scope of variable, eg P for public, PRG = first three characters of the source file that created the variable, NAME = variable name in the remaining 6 characters that dBase/Clipper allowed.

The first 4 and last 4 lines of the source code were 80 * long. Why? So he could hear the dot matrix printer starting and finishing the printing of a file. Memory is the entire program was printed via the mainframe weekly, 20,000 pages.

I'm sure there were many more that I've managed to dump from my brain.

I was a very new self-taught programmer at that stage but knew enough not to listen to the mad scientist and get the hell out of there before I asked to take over the project.

And yes we told management how bad these practices were but always got the usual "were paying this contractor top dollar he must know what he's talking about".

Having an audio marker so you know when each file is printing is ingenious. I'm going to add \07 to the start of each file now.
–
configuratorSep 29 '10 at 4:20

2

Using a naming scheme like this (Not the upper case) made some sense as dBase's variable "scoping" rules were non-existant. Everything was effectively global. An i used to index an array in one procedure could interfere with an i in a calling procedure. You need to use PRIVATE ALL LIKE m* and PRIVATE i to prevent this "shadowing"
–
GerrySep 30 '10 at 1:12

There will be no code written using interpretive languages because I lost 25 million on that {expletive} project written in Java.

The Java project was a stock trading system designed to handle a few dozen stocks, that was now being used to process thousands. Instead of addressing the design flaws or poor hardware, the whole company was forced to convert all non C/C++ applications to C/C++, and all new development had to be in C/C++. Interpretive languages meant anything not compiled, and the owner only considered Assembler, C and C++ compiled.

For an 800 person company, in which most of the code was in Java and Perl, this meant the whole company spent most of their time over the next couple of years rewriting perfectly fine code in C/C++.

Funny enough, some twenty years before this fiasco, I was at another company in which the tech lead decided that our sorting logic (it was a Bubble Sort) needed to be recoded in assembler instead of being replaced by Quick Sort because -- Algorithms do not improve performance. The only way to improve performance was to rewrite the same logic in assembler.

Like a lot of programmers (but not enough), I hate code decoration. It infuriates me when I have to use a dollar sign($) prefix for variable names, or underscores for private variables, even with no getters/setters. If you need to decorate you code to understand it, then you need to get the hell out!

I don't think grouping your variables together in your favourite proprietary IDE is a good enough reason to deface all your code.
–
Adam HarteSep 9 '10 at 20:44

1

+1 Using a good IDE (one that can use regex search) makes more sense to me. Scratch IDE, learn to use a text editor and terminal and you will be a much better programmer. As a side note, I don't particularly like the perl sigils, but at least they have a use, unlike the PHP ones.
–
alternativeSep 9 '10 at 22:32

6

Sigh... another one of those "IDE's are for pussies" people.
–
NailerSep 23 '10 at 13:41

I've been working with a web system for a while where all parameters passed had to be named P1, P2, P3 etc. No chance in hell to know what they where for without extensive documentation.

Also - although not strictly a coding standard - in the same system, every single file was to be named xyz0001.ext, xyz0002.ext, xyz0003.ext, etc - where xyz was the code for the application in itself.

This was a LONG time ago -- 1976 to be exact. My boss had never heard of Edsger Dijkstra or read an issue of CACM, but he had heard a rumor from somewhere that "GOTO is bad", so we were not allowed to use GOTO in our COBOL programs. This was before COBOL added the "end if", so at the time it had only two-and-a-half of the three classic control structures (sequence, if / then / else, perform (i.e. do while)). He grudgingly allowed GOTO in our Basic programs, and branch instructions in our Assembler language programs.

Sorry that this is a sort of "you had to be there" story. As far as I know, every language invented since 1976 has adequate control structures so that you never need to use GOTO. But the point is, the boss never knew WHY GOTO was considered harmful, or which language was the infantile disorder and which was the fatal disease.

You'd be hard pressed to return something from a function declared as void.
–
Mircea ChireaSep 9 '10 at 18:16

7

@MAttB Consider under what conditions the final (else) branch would be taken.
–
RichardSep 9 '10 at 19:52

6

else { return string.Empty; } will execute when the above 2 lines have been edited by a maintenance developer 5 years from now. However, returning string.Empty will hide the fact that is used to be an impossible condition. It should instead throw InvalidOperationException("This code wasn't intended to support three value logic");
–
MatthewMartinSep 10 '10 at 19:23

What was wrong with the project lead's HeapSort?
–
7wpSep 9 '10 at 18:02

4

Actually if code accepted external user input QuickSort may be wrong as it opens to O(n^2) DOS attacks (feeding worst-case input). Also why it wasn't possible to switch - it itself was valid excuse of not using STL.
–
Maciej PiechotkaSep 9 '10 at 22:17

I worked for a short time in Japan. I was doing complex mathematical coding. The company coding standard was to have absolutely no comments. It was difficult as I would have liked to add some comments to explain the complex calculations and not forget myself after few weeks. Pity the next guy who comes after me to understand what the code was doing.

It was the first time I ever saw that coding comments were prohibited.