Code Formatting - A reflection of Coding Level?

[Ajit Jadhav]

Yes! ( :mad: + :eek: + X| + :(( )/4 (There is no single emoticon to indicate the effect of poorly formatted code on a maintainer/enhancer.) I have seen code that had, in the same .cpp file, the following: - Function opening brace begin anywhere starting at 0-8 spaces and/or 1/2tabs (making only some of them to appear at 4/8/16 characters as you go on changing the editor setting in frustrated attempts to get it right). - Variables are declared at any positions starting from 0 to 8 characters. Tab characters appear infrequently. - No comments on top of any function. Some commented out code within functions. Some old, irrelevant comments within functions. No separate Word documentation that is also applicable to the .cpp file. - The call depth in that part of code was five/six. The total call-depth would probably be 20+, not counting the incursions into user, GDI, winsock, shell, etc. A callback function and a non-prototype-declared (static global) function sat on top of those five/six member functions. - Of course, implementation of several big classes appeared in the same file. - No Hungarian notation followed. Same varibale name used to denote a VARIANT, a BSTR, a CComBSTR, a COleVariant and a char* in all these, closely related, functions. - Same variable name used for arguments and locals in the closely related functions. - Function closing brace ends at 2 chars space, mostly. The range is again 0 to two tabs. I was supposed to maintain that, as a temp contractor brought in to fix bugs. VC++ IDE facilities did help to an extent (to correct the sheer formatting nonsense) but no more. No documentation. When I protested, my manager arranged for "only one 15 minute meeting" with "our senior engineer who wrote this is now very busy with important projects at this point of time". (Who promoted?) The manager continues: "I am doing this for you because I want to have that bug fixed on priority. He will help us on my personal request. But it's unofficial. He is not a resource on this project any longer. So don't don't talk about it and don't waste his time." (Must be rocket science, what he is doing now!) The senior arrived. Shake-hands are ignored by all but a vague smile does precede the talk. "This should be pretty simple code to understand for an experienced programmer like you...." (defensively sweet tone). VC++ objects browser is launched. "There have been many people working on this part of the code, and they used different editors. We don't have a policy of imposing any one editor on peop

[Nick Hodapp]

Oz, since you've seen my code I'm now curious how I rate... ;P

[Oz Solomon]

Is that a trick questions? It sure sounds like one! ;P -Oz --- Grab WndTabs from http://www.wndtabs.com to make your VC++ experience that much more comfortable...

[Farhan Noor Qureshi]

I couldn't agree with you more. I am one of many unlucky soles who have to read/modify/enhance *very* badly formatted code. :) ;) ;P :-D :cool: Farhan Noor Qureshi

[Ravi Bhavnani]

Cleanly formatted code with useful variable names is always easier to read and understand than code that is poorly and/or non-uniformly formatted and that has odd variable declarations. Stuff like this gets caught at our code reviews, where the author is given a gentle nudge to observe our coding guidelines. Another positive outcome of our code reviews is that if I have to explain to two other seasoned developers what's going on in my code, it probably could stand to be better commented and/or rewritten. /ravi "There is always one more bug..." http://www.ravib.com ravib@ravib.com

[William E Kempf]

I disagree. The VC IDE does a fine job of formatting. Your complaints actually reduce down to you simply preferring a different style... but that's a religious debate that's been raging since the advent of computers. To address some of your complaints directly: > All variables at the top of the function, one variable per line. In C this is fine. In C++ this is a bad idea. Many declarations result in very expensive calls to constructors. If the flow of control dictates that a variable may not even be used declaring it at the top of the function results in unnecessary overhead. In C++ you should declare variables as close to first use as possible, and not before. > Variables other than counters named using Hungarian notation. An all together different religious debate. Hungarian notation in C++ serves very little purpose, and I've never seen it used consistently enough to be of any real help. Even the MS code (they were responsible for the advent of HN) uses HN inconsistently, at least partly due to legacy trivia (such as changes in the actual types when the switch was made from Win16 to Win32). I'm not going to try and convince you to change your preferences here, but there's a HUGE difference between poorly formatted code and code using a format different from yours. William E. Kempf

[Ed Dixon]

Agree. I think there are two related issues here: code formating and code commenting. It always works best for me if both are well done. I really dislike looking at code when comments are crammed in, no spacing between lines, etc. I also dislike code where someone has found cleaver ways to put the equivalent of two simple statements into a single statement. Yes it probably runs faster, but is almost always harder to read. Given the choice I'll almost always trace performance off for more understandable code. Ed

[Stuart van Weele]

My previous message was poorly formatted itself. Things I find annoying about the VC IDE wizards include: Not separating wizard created functions and variables. Create a few variables and functions and see how they are laid out in the header file. Interleaving public, protected and private areas in the header file. Sticking the entire CAboutDlg class at the bottom of the app classes implementation file. Sticking the implementation of some class constructors in the header file. The list of my personal style preferences was just that - my personal style preferences. I don't expect the IDE to implement them, although it would be nice if MS followed the Hungarian notation conventions it suggests for others.

[Rick York]

I agree too. I have noticed that I spend much more time reading code than writing it so I try to make it as easy to read as possible. I have also noticed that different people have different definitions of what easy to read means. ;)

[Robert Dickenson]

Well I have noticed that the developers who contribute articles to this site have always presented cleanly formatted code. Since we don't see poorly presented code here one must assume that those who don't care about the presentation of their work never have anything worthy to offer to the rest of us. One of my worst habits when taking over existing code is that I always feel the need to reformat and tidy up its appearance before I can *really* get in and read/comprehend what the code is saying. Poorly presented code is simply harder to read. :eek: Over the years as I have worked for many different clients I have always been presented with a new set of 'rules' on code formatting, usually called a 'style guide'. Since everybodys idea of style tends to be significantly different I now have a very inconsistant looking code base, which really bugs me. I would like to see the 'gurus' define THE STYLE GUIDE which I would adopt religiously, and preach it to the unconverted. I don't mind changing the style I write too, but I do hate having to change it every week. X| Regards, Robert Dickenson.

[Stephen Kellett]

Some of my pet peeves are not coding in such a way as to help yourself/others when debugging. Simple stuff like the following:

void *someFunc() { return aValue;)

This is really badly written. If put a break point on it you can't see the the value of 'aValue'. Why? Becasue the stack frame is not set up. To set the stack frame up you have to switch to assembly view, and step until the stack frame has been set up. Slow, cumbersome, thoughtless. Writing it...

void *someFunc()
{
return aValue;
)

solves this problem. Its a trivial fix which saves an enormous amount of time. Also not declaring constructors and destructors even if they are not used. These are often great places to put breakpoints. Likewise they should be written so that the code for them is not inline. So that editing them to put debugging code in doesn't touch the header file and cause a huge rebuild. Again trivial stuff, but a lot of people just don't get it, :-(. End Rant Stephen Kellett -- C++/Java/Win NT/Unix variants Memory leaks/corruptions/performance/system problems. UK based. Problems with RSI/WRULD? Contact me for advice.

[Daniel Smart]

You've touched on one of my bete-noirs. If a function *isn't used* then it shouldn't exist. If the default constructor/destructor etc is correct for a class, then you shouldn't rewrite it. When I'm reading your code, if I come across a replacement for one of the defaults, which provides exactly default behaviour, I'm left wondering whether you intended non-default behaviour and made a mistake, don't understand the default behaviour, or are mindlessly following silly coding rules. If later I find a constructor that differs trivially from default behaviour, I now have no idea whether you intended default behaviour, or not. Furthermore I would suggest that if someone writes code where setting breakpoints in default constructors/destructors is essential to debug the code, then you have far more serious problems than code layout. Dan

[Zyxil]

EXACTLY!!! if you code within a group of people and they can't read your code, then you are a drag on the group's efficiency and productivity. cleanly formatted code is the MINIMUM of professionalism. anything less is unprofessional hack code that needs work. my code standards for people i mentor or work with include large amounts of formatting and naming. my standards aren't nuts or 100% anal about "put a space there", but if you don't take the time to learn the easy habits of code formatting, what other more iportant code habits have you slacked off on? -John

[Erik Funkenbusch]

I disagree. The reason is that I've been on the reading end of many programmers code, usually in debugging. The ones that are obsessive-compulsive about prettifing their code tend to spend more brain power on formatting the code than actually programming, and tend to make STUPID mistakes. Although it is indeed easier to fix these bugs because of their style, I don't find that neat programming leads to less bugs or better code. Also, sloppy code doesn't mean worse code, since it can be a reflection of many things. For instance, they may not have the time to make it pretty. I know one employer that thought that if the code looked too good, it meant you had too much time on your hands. Finally, some people do have a consistent style, that is easy for them to read and understand, but not so easy for other programmers to read and understand. They write better code in their own way, and would produce worse code if forced to do it in a different way.

[Jim A Johnson]

I think the only thing this thread proves is that generalization is always a bad thing. That said.... sloppy code indicates sloppy thinking. Everyone makes mistakes. Consider: - If you don't have time to make the code neat, you probably don't have time to make it work right. Doing code in a hurry is a recipe for error. - Making bugs easier to find and fix results in fewer bugs. How can anyone argue that point?

[Erik Funkenbusch]

I disagree completely. "neat" is a subjective quality. If someone has to make their code conform to some "neatness" standard than is different from their own, that obviously takes a lot more time than if they use their own. Further, have you never heard of the term "conservation of energy"? I knew this guy with a desk piled high with stuff that nobody could find anything in, yet if you asked him, he knew where everything was and could find it in an instant. He didn't waste time organizing and quantifying, but rather simply remembered where everything was, conserving energy which he used to great effect elsewhere.

[Stephen Kellett]

Furthermore I would suggest that if someone writes code where setting breakpoints in default constructors/destructors is essential to debug the code, then you have far more serious problems than code layout. Indeed. Code layout is part of a larger job of thinking about minimising the cost of code maintenance. If adding a destructor or default constructor to a class definition is going to cause a large rebuild (lets say most of the files in most of the 50+ DLLs of 2 million lines of code - and yes I am working on code where a few files are sadly this important - (*)), then having that constructor/destructor always defined (even if #ifdef _DEBUG conditionally compiled) is a major bonus as I don't have to wait a few hours whilst it builds. I guess I got slightly off topic, but it is these trivial things that do matter. Default constructors/destructors that are supplied by the compiler are identical to empty constructors/destructors, so if I found them in someones code I'd assume they are there for a reason, not because of a mistake. I guess we have different attitudes to /expectations of the rest of the team we work with. Besides a decent comment with each would put your mind at rest in any case. (*) One part of my gig with these people is to track down resource and performance problems that the rest of the team are too busy to look at. As a result I'm often dumped into code I've never seen before. In those cases constructors and destructors and lifetimes of said objects are often a great help in determining at what point something didn't go as planned. Stephen Kellett -- C++/Java/Win NT/Unix variants Memory leaks/corruptions/performance/system problems. UK based. Problems with RSI/WRULD? Contact me for advice.