Code Cleanliness

#1 I normally use tabs to indent, a single tab per level. I find it faster than 3 spaces, and sometimes time counts :)
As for the this versus that you posted I find the second block of code more unreadable than the first one. This would be more readable:

// Prep scan values for averaging
      int scan_alpha_t         = left_scan_edge_alpha_z;
      left_scan_edge_alpha_t  += delta_left_alpha_z;
      int  delta_scan_alpha_t  = (right_scan_edge_alpha_z - scan_alpha_z) / rectSize;
      right_scan_edge_alpha_t += delta_right_alpha_z;

And that's what I do when I have many assignments together that have something in common.
Also I put comments there time by time, specially if there's something that's not really clear, because sometimes it's very stupid to add a comment like this:

// Render the box
   renderBox();

Doh! The method itself looks like a comment :)
I sometimes don't realize and do that though. Depends on the mood... hurry, unsure, serious, whatever.

I think TGE should have some nice coding guide lines that people should try to follow, but since TGE is already a complete mess It wouldn't be correct to impose a coding method when the engine itself doesn't even follow it.

#2 Ben,

I've always used a 4 space tab as I find that 3 indents are not obvious enough and leads to condensed looking code. This probably stems from the fact that I always develop on systems running at fairly high resolutions (at least 1280x1024 and normally higher).

Xaviers' example is pretty much what I do e.g. align similar statements column-wise...

// Setup Our Viewplane.
	ViewPlanes[0].set(CameraPosition,       FarPosLeftUp,           FarPosLeftDown);
	ViewPlanes[1].set(CameraPosition,       FarPosRightUp,          FarPosLeftUp);
	ViewPlanes[2].set(CameraPosition,       FarPosRightDown,        FarPosRightUp);
	ViewPlanes[3].set(CameraPosition,       FarPosLeftDown,         FarPosRightDown);
	ViewPlanes[4].set(FarPosLeftUp,         FarPosRightUp,          FarPosRightDown);

I hate the space-saving practice of putting the starting-brace on the same line as a loop condition e.g.

if (timeMultiple < 0.01 || timeMultiple > 100) [b]{[/b]
      Con::warnf("ParticleEmitterNodeData::onAdd(%s): timeMultiple must be between 0.01 and 100", getName());
      timeMultiple = timeMultiple < 0.01 ? 0.01 : 100;
   }

Is space that important?

Don't be scared to over-comment as per Xaviers' 'RenderBox' example (I'm guity of doing it all the time). I've yet to be convinced that such commenting is bad for anyone. Even if it doesn't bring any more information, it allows for a nice seperation of code. For instance, prior to the actual call there would, more than likely, be a big block of processing leading up to the call itself. Seperating the call with a tiny comment can clarify that it is the end result or is some kind of functional milestone. Vertical code seperation, in my opinion, if often overlooked but it can be more important than horizontal seperation when it comes to readable code.

I've started adding prefix/postfix spaces around conditional statements to make them stand-out e.g.

if (Parent::preload(server, errorBuffer) == false)
      return false;

...becomes...

if ( Parent::preload( server, errorBuffer ) == false )
      return false;

I normally have '#defines' as all uppercase and can use underscore in them. Variables have initial-lowercase followed by word-capitalised and definately no underscores e.g.

#define WRITEPLAYERLOG
#define BUY_BEN_XMAS_PRESENT

S32 countThePresents = 10;
F32 jingleAllTheWay  = 123.45f;

There should be at least a line seperating functions within any file and preferably a couple of lines describing the functional intent and the authors initials. Also, try to make the seperators a standard length e.g. 40, 60 or 80 characters e.g.

//--------------------------------------------------------------------------

...or...

//--------------------------------------------------------------------------
// Do a Quicksort on the particles so that transparency works.
// MM: 16/11/03
//--------------------------------------------------------------------------

All variables/functions within a class-header should be sorted. Mixing-up these within a header makes locating stuff dificult without support from the IDE.

I shall leave it there otherwise it may sound like ranting... :)

- Melv.

#3 It's true about vertical spacing, and as I said, I think it's stupid to comment with obvious stuff but I do it all the time, it's kinda inside my hands ;)
I also separate, if not with comments, with blank lines all the code 'blocks' that do similar things.

About the braces, I only put them next to the if statement when it's a short thing, like for example:

if(player.isObject()) {
        return;
    }

In those cases I find it easier to read, since you can see the ending brace at the same time, so you can't get lost. But when you have bigger if's ... 5,6, 10 20 lines I use braces in the same way Melv does

if (blah)
    {
        ......
    }

The only thing in which I disagree is in the adding of extra spaces between parentheses and conditions, I always hated that, and I always remove them when I see them :)
I think the parentheses are explicative enough, I mean, you can't use parentheses for anything else, so you do know that's a parameter or condition.

Another thing I've noticed sometimes people do is adding the command for single line if's after the if itself, not below, for example:

if(weAreDead()) return false;

Sure it saves a line and it's pretty obvious what it does, but when you have your eyes used to seeing all the conditions below an if that makes it anti-intuitive to read.

The separator lines are always a good idea and I use them in any code I do, big or small, torque or not.

Last but not least, I also use uppercase defines with underscores, but I almost always start using an underscore, like this:

#ifndef _MAIN_H_
    #define _MAIN_H_
    ....
    #endif

That would be a standard header definiton in many of my apps/code. I just checked and Torque uses an underscore as the first char too.

Oh, and I also use a lower case first char for methods and variables, and normally prefix an "m" to members of a class following by an upper case letter, that helps a lot on the readability, not because I wouldnt remember it's a member, after all, in TGE *everything* is a member, but it helps because when your eye is trained to see an mXXX you automatically catch that as a variable stored in this object.

player->myMethod();
    float mFloat;

    // Not intuitive:
    player->MyMethod();
    float myfloat;

At least for me it's obvious that the first two look nicer, and have more eye candy on em ;)

#4 Man, I hope this doesn't turn into a Religious Issue - I've seen comments and space -vs- tabs turn into near war on the SWEngGameDev list before (I say near, because I normally kill the subject quickly :-)

I use Tabs - that way, anyone can pick thier view in any editor :-) Plus, as Xavier said - it's faster, and you can do things like put an if structure in front of some code (in VC++) and highlight the rest of your code and press Tab to move it all over, or later if you take the if back out, hit shift tab. I'm sure there's a way to do it with spaces, but, I never learned it ;-)

I'm also with Melv on { }'s - it doesn't belong at the end of the if, it belongs on the next line :-) To my eyes, it just help's make the structure that much more readable. But that's just me I'm sure.

I sort of agree with Melv about separators - except I'm the worlds worst about actually putting them in there!

All comment's should follow exactly what Melv says - that's what I do these days :-) However, I take it a step further, just to make life easy for me when doing things like merging with HEAD:

if (timeMultiple < 0.01 || timeMultiple > 100) {
      // DRSJR:  Removed console warning - does the end user care?
      // Con::warnf("ParticleEmitterNodeData::onAdd(%s): timeMultiple must be between 0.01 and 100", getName());
      
      timeMultiple = timeMultiple < 0.01 ? 0.01 : 100;
   }

By doing that, whenever I do a merge with HEAD or something, I can just search out my comments, instead of plowing through the diff output, wondering what the heck changed. (I'm not the worlds most efficent when using WinMerge - I'm sure there's easier ways to do it.) Later, if I add a resource, I do similar (if thier resource wasn't already commented that way) so that I can track down problems that might have been caused by the resource.

I also do tabs within similar code sections, like Ben mentioned, but only if I think about it :-)

Code Cleanlyness is a good thing. Anything that helps readability or understandability of a piece of code never hurts anything. I'm guilty of writing some ugly stuff that could easily be cleaned up and made more readable - but I always appreciate when I deal with someone else's code and it's nicely cleaned up :-)

#5 I could care less about:

if(weAreDead()) return false;

-vs-

if( weAreDead() ) return false;

To me, it doesn't matter much, just as long as it's consistent throughout the project.

#6 Guys,

A line from a book on this very subject read, "It's better to have questionable guidelines for code layout than to let chaos rule by having none."

As long as they're only guidelines then everyone's happy ... but you will get a severe beating if you don't do it like we say! ;)

- Melv.

#7 The thing about coding style is that every experienced/professional programmer has their own and are fiercly protective of it and fiercly anti anyone elses, even if that other style is 99% the same as theirs. Beginners just sit back and look bemused while they realise they havent yet got a style and have just been borrowing someone elses while they learn ;-)

With any project, as Melv said, the only thing that really matters is that the style is consistent throughout. At the end of the day, who really cares *how* it's written, it all does the same thing in the end.

Personally, I think coding style is just another form of programmer procrastination... we'll do *anything* (usually argue about something completely pointless for hours) rather then actually do some work ;-)

PS. Spaces are evil, tabs all the way! :)

#8 Cool thoughts, guys. Tom and Melv (and whoever else agreed :P) are exactly right - the important thing is a consistent style, not necessarily the "right" style. (Which is why I'm going to replace all spacing and newlines in Torque with macros so you can tweak it via preprocessor constants to be "right"... :P)

I'm a little curious, though, as to why y'all don't eschew tabs. I've seen some ugly, bad things happen because of them. You gain a little speed, but if you move to a different editor, you're screwed until you figure out how to get the tab sizes right again. And if you're working with multiple people, unless everyone uses the same size tabs, you'll never get alignment quite right - it's usually necessary to add a few spaces to the end of a set of tabs to get the alignment just right, and of course, if you change tab sizes, then you have to tweak the spaces.

And heaven forbid you should try to tab align things inside of string constants. 0.o

How do you guys work around those problems? Or is it not a problem? Do you often work in environments like Torque, where there are a large number of people working on/looking at the code?

Um. Only other datapoint I have is that I run at 1600x1200 and am usually content with 3 space tabs. Maybe that's just cuz I have young eyes. :P

#9 I think that tabs spaces are left to the editor and user, for example, you like 3 spaces, Melv likes 4. Well then if we had everything using tabs you could set your tab space to 3 spaces and Melv would set his to 4 spaces and everyone is happy!
I haven't really seen big issues with tabs, and I never cared how they 'look' on other editors since it's tweakable, I do care to know that there's a tab character right there. (Unless my editor is tricking me there is one ;) )

#10 Xavier,

Well, no not really, because... say I have a list of members:

8 space tabs:

        bool                  isBacklogged();
        virtual void          getMoveList(Move**,U32* numMoves);
        virtual void          clearMoves(U32 count);

4 space tabs:

    bool          isBacklogged();
    virtual void      getMoveList(Move**,U32* numMoves);
    virtual void      clearMoves(U32 count);

3 space tabs:

   bool        isBacklogged();
   virtual void     getMoveList(Move**,U32* numMoves);
   virtual void     clearMoves(U32 count);

Notice how the alignment degrades. It only gets worse if you have alignment on more than one column (say you want your comments aligned, too, which I usually do).

I think Melv is spot on about commenting key method calls. I hate it when there are long groups of setup code for a single call (like happens in the scenegraph code), and then you miss the key call cuz it's uncommented in a big block of other stuff...

#11 Ben, you should read Code Complete by Steve McConnell. He's got a whole chapter just on layout; it's pretty interesting. There is a bit in there called the Fundamental Theorum of Formatting and that is that "good visual layout shows the logical structure of the program". "Making the code look pretty is worth something, but it's worth less than showing the code's structure." That's what he focuses on, different ways to show structure better with layout, and a lot of what he's saying is from actual research.

Ie - "Subjects scored 20 to 30 percent higher on a test of comprehension when programs had a 2 to 4 spaces indentation scheme than they did when programs had no indentation at all. The same study found that it was important to neither under-emphasize nor over-emphasize a program's logical structure." "The second lowest [comprehension] were achieved on programs that used six space indentation." "Interestingly, many subjects in the experiment felt that the six space indentation was easier to use than the smaller indentations, even though their scores were lower. That's probably because six-space indentation looks pleasing."

My 2 cents are that it's best to have general guidelines that the team mostly follows if not follows to the letter. If everyone has drastically different styles it sways a little to the "code ownership" demon. If everyone follows the same guidelines, people get less touchy about someone else reformatting "their" code.

It also helps to think about other people trying to read or modify your code when you're writing it. Like McConnell says, clean it up so it's readable, but make sure it's logically grouped and indented, not just formatted so it looks pretty.

Here's something some of you might find amusing. I've been using Visual Slick Edit recently and I noticed there's a menu option "Beautify Source". It will go through and re-bracket all the code so whatever style you like, with spaces or no spaces between parens and everything. Funny as hell - reformat the whole file to how you like it in one click. The only problem is when you do a diff, there's an insane number of changes, so anything else you changed is lost in a sea of indentation, bracket, and parenth changes.

There was actually a coding guideline that we used for coding on Tribes 2 - anyone know if that document is in the Torque somewhere?

#12 Hey, Brian. Nice to see you saying stuff. :)

Do you happen to own that book? Maybe I can borrow it from you sometime.

Sounds like McConnell is spot on.

No, I don't think that that document is in Torque anywhere. :( Though it might be a good idea to add some documentation on the topic. I think Tim mentioned a few things on the patching info page, but I don't think there's an "official code layout guidelines" anywhere.

#13 @Ben: You definately want to read "Code Complete", it's certainly a good book. Hmm, Visual Slick Edit, I'll have to look into that one.

- Melv.

#14 @Ben: good point. :)

#15 @Ben (delayed),

I set all my editors to default to 4 space tabs for most languages and 8 for assembler or plain text. The main reason I dont like spaces is it's harder to change the spacing if I ever need to and using the arrow keys doesnt skip a whole indent, which I've kind of gotten used to.

In terms of teams, as long as its all agreed on beforehand its usually not a problem. Which reminds me, BoomBall is using tabs set to 4 and I havent mentioned it to anybody ;-) Funny how the little things get overlooked when you have a really tight schedule.

T.

#16 Hey Ben, yeah, I'll loan it to ya when you come out. Check your local bookstore too, you'll probably want to buy it just looking at the table of contents ;)