Coding style question

[aaaaa00]

Hungarian notation stinks. It's use simply means your platform IDE sucks (e.g. Visual Studio, yes, its code editor does suck). The various arguments for why it should be used (human contextual understanding, safer renaming of variables) melt away if your editor does incremental parsing, compilation, and supports refactoring.

In my editor, members, statics, locals, classes, etc all have different styles and colors. And renaming a variable never causes an error, ever, because the editor performs inferences types across the whole project, and verifies that a manual rename is correct.

My editor does all of that (Source Insight), and I stil find Hungarian easier to deal with than non-Hungarian code.

The reason is that everyone uses different editors because they like different tools. Whether I do a code review on my machine, or on someone else's machine, with someone else's editor (VI, emacs, Visual Studio, or god-forbid NOTEPAD.EXE), properly and consistently applied Hungarian works equally well.

However, I agree that for managed code or higher level languages like Java, or if you can force everyone on your team to use the latest whiz-bang uber-IDE, Hungarian is much less useful.

 

[DemoCoder]

Well, I'd either put my foot and require people to use better tools in my team, or I'd do code reviews by checking out code onto my own machine and reviewing it in my editor.

I think anyone writing significant amounts of C++ code these days in VI is both insane and a liability to the project, no matter how good they think they are, it will be a drain on productivity.

 

[Simon F]

I think anyone writing significant amounts of C++ code these days in VI is both insane and a liability to the project, no matter how good they think they are, it will be a drain on productivity.

So ... "ed" is out as well?

 

[Basic]

Here's one that you don't hear often, but it's still important:

Never ever put a tab in a source file.

It's quite irritating that many editors insert tabs per default. Good thing that all editors I've seen can swtch to use space instead.

 

[Humus]

And why would that be? I find tabs makes a lot more sense than using spaces. If you prefer say 3 steps instead of 4 it will look correct without changing the source. And navigating through tabs is quicker and a lot more intuitive than spaces. Space indented files are often inconsistent (like most indepentations 4 spaces, but some 3 or 5)

 

[Rodéric]

I agree with Humus, TABS are great, they allow each user to choose how many spaces will be shown in their own editor and so it's much more flexible than inserting spaces...

 

[Simon F]

Ditto. I've no problems with tabs.

 

[Nick]

Tabs for me. I don't see any real advantage of using spaces.

 

[Xmas]

I use tabs as well. And spaces to align multi-line expressions starting from the tabbed indentation level, since that requires a fixed width.

 

[Rodéric]

For Nick, from the optimisation thread:
the sample code I wrote wasn't from my code, I just typed it quickly to show the idea nothing else, some real code would rather be :

unsigned int uiIndex;
for ( uiIndex = 0; (uiIndex < m_uiCount) && (m_aObjectList[ uiIndex ] != pObject); ++uiIndex );
return uiIndex;

The 'ui' subscript is used to indicate data type.

I usualy create a uiCount value for the loop previous to entering it, feels clearer, may avoid useless function calls too.
(such as calling getSize() or size() or getXXXCount())

unsigned int uiCount = maxvalue/16;
for ( unsigned int i = 0; i < uiCount; ++i )
{
...
}

 

[Xmas]

But shouldn't it be m_apObjectList then? It is not at all obvious that m_aObjectList[ uiIndex ] is supposed to be a pointer to an Object.

 

[Basic]

That's true when one tab == one indentation step. Some editors enforce that, some don't. It's common that a tab is 8 spaces, while an indentation step is 2 or 4. So a couple of indentation steps becomes one tab.

I've seen several pieces of bought source or code examples (from outside my company) at work. Every single one that included tabs had these errors and/or a mix of tabs/spaces for indentation. Resulting in a screwed up indentation if you don't use the same tab length as the writer.

I don't have a (big) problem with code with one tab == one indentation step. As long as it's strictly enforced as one tab == inside one curly (or on a curly-free line after an if/for/...), and never more or less than that. But it's very easy to accidentally use the wrong kind of whitespace somewhere and it's hard to spot that when coding. (Many editors have a "visibles" mode that show the whitespaces exactly, but it's usually to cluttered to use all the time.)

[COLOR=Black][COLOR=Red]--->[/COLOR]if(expression1 ||
[COLOR=Red]--->[/COLOR]   expression2   )
[COLOR=Red]--->[/COLOR]{
[/COLOR][COLOR=Black][COLOR=Red]--->--->[COLOR=black]// Lots of arguments to this function.[/COLOR]
[/COLOR][/COLOR][COLOR=Black][COLOR=Red]--->--->[/COLOR][/COLOR][COLOR=Black]Func(argumentA, argumentB,
[COLOR=Red]--->--->[/COLOR]     argumentC, argumentD,
[COLOR=Red]...[/COLOR]
[COLOR=Red]--->--->[/COLOR]     argumentZ);
[/COLOR][COLOR=Black][COLOR=Red]--->[/COLOR]}
[/COLOR]

If the editor made sure that you don't break this rule, then OK. But then again, if the code moves to someone that doesn't have that feature, you're in trouble again. Most effective way is to not allow tabs in the source.

If there's short tactical comments (short comment on the same line) in the code, I want them vertically aligned to reduce clutter. Not neccesarily on one exact column over the whole file, but the column should not change every other line. Varying tabs would break that.


I looked at your framework source Humus, and you are rather consistent with the tabs, following the rule I see as OK. But there's still places were you indent with just spaces. And you've kind of circumvented the problem in multi line function calls by never doing any, resulting in lines with sometimes >200 characters.
In some of the headers for libs you're using (in Audio and Imaging) there are tabs in the middle of lines, resulting in unaligned comments (that were supposed to be aligned). But that's not your code.


Then there's the problem (here at work) that some people like to print code snippets for review. All such printing seem to end in 8 spaces per tab. And 8 spaces per indent looks rather bad. But I see that as a problem with the people who like to print it, rather than with the tabs. :smile:

 

[Nick]

If the editor made sure that you don't break this rule, then OK. But then again, if the code moves to someone that doesn't have that feature, you're in trouble again. Most effective way is to not allow tabs in the source.

That's replacing the evil with a lesser evil. Not solving the actual problem.

I don't think it's unreasonable in 2006 to expect people to work with editors suited for coding. For practically any platform there are free or very cheap good IDE's. I don't want to touch my code quality just so that someone with an outdated editor is able to work with it half efficiently. If they want to continue to work that way then they're going to have to rewrite the code to their likings or use scripts/tools to do it for them.

 

[Nite_Hawk]

That's true when one tab == one indentation step. Some editors enforce that, some don't. It's common that a tab is 8 spaces, while an indentation step is 2 or 4. So a couple of indentation steps becomes one tab.

I've seen several pieces of bought source or code examples (from outside my company) at work. Every single one that included tabs had these errors and/or a mix of tabs/spaces for indentation. Resulting in a screwed up indentation if you don't use the same tab length as the writer.

I don't have a (big) problem with code with one tab == one indentation step. As long as it's strictly enforced as one tab == inside one curly (or on a curly-free line after an if/for/...), and never more or less than that. But it's very easy to accidentally use the wrong kind of whitespace somewhere and it's hard to spot that when coding. (Many editors have a "visibles" mode that show the whitespaces exactly, but it's usually to cluttered to use all the time.)

[COLOR=Black][COLOR=Red]--->[/COLOR]if(expression1 ||
[COLOR=Red]--->[/COLOR]   expression2   )
[COLOR=Red]--->[/COLOR]{
[/COLOR][COLOR=Black][COLOR=Red]--->--->[COLOR=black]// Lots of arguments to this function.[/COLOR]
[/COLOR][/COLOR][COLOR=Black][COLOR=Red]--->--->[/COLOR][/COLOR][COLOR=Black]Func(argumentA, argumentB,
[COLOR=Red]--->--->[/COLOR]     argumentC, argumentD,
[COLOR=Red]...[/COLOR]
[COLOR=Red]--->--->[/COLOR]     argumentZ);
[/COLOR][COLOR=Black][COLOR=Red]--->[/COLOR]}
[/COLOR]

If the editor made sure that you don't break this rule, then OK. But then again, if the code moves to someone that doesn't have that feature, you're in trouble again. Most effective way is to not allow tabs in the source.

If there's short tactical comments (short comment on the same line) in the code, I want them vertically aligned to reduce clutter. Not neccesarily on one exact column over the whole file, but the column should not change every other line. Varying tabs would break that.


I looked at your framework source Humus, and you are rather consistent with the tabs, following the rule I see as OK. But there's still places were you indent with just spaces. And you've kind of circumvented the problem in multi line function calls by never doing any, resulting in lines with sometimes >200 characters.
In some of the headers for libs you're using (in Audio and Imaging) there are tabs in the middle of lines, resulting in unaligned comments (that were supposed to be aligned). But that's not your code.


Then there's the problem (here at work) that some people like to print code snippets for review. All such printing seem to end in 8 spaces per tab. And 8 spaces per indent looks rather bad. But I see that as a problem with the people who like to print it, rather than with the tabs. :smile:

I also prefer spaces, and for many of the reasons you mentioned. I also will occasionally need to edit source files using text editors and prefer that the code look the same in any editors I might use as it does in my IDE.

As far as optimizations go, I'm almost always in favor of waiting on code specific optimizations until you know that it is going to be a problem. Far too often you end up spending time optimizing the wrong things or wasting time on things that provide little gain. It can also obfuscate the code, making it harder to identify algorithmic optimizations that would yield much greater improvements.

Nite_Hawk

 

[Basic]

I don't think it's unreasonable in 2006 to expect people to work with editors suited for coding.

I admit that I might not use the latest editor. But I haven't seen help for tabs like I described them in either of Emacs, CodeWright, VC++6, or Eclipse. Can modern editors handle when to insert space and when to insert tabs? Ie, the multiline problem, and insert space if there's any non-ws before the cursor when pressing the tab key. And do they keep the ws in a good form through deletes, and cut&paste.

 

[Rodéric]

But shouldn't it be m_apObjectList then? It is not at all obvious that m_aObjectList[ uiIndex ] is supposed to be a pointer to an Object.

Indeed you are correct, I should have typed m_apObjectList ^^

 

[Frank]

How about the most important argument to only do one operation per line: to make it really easy to debug. Step through, and hover the mouse to see the value you want. No need to make a watch for each and every thing. And I always use brackets or begin..end, needed or not. Makes it easier to follow the flow of the program IMHO.

For the Hungarian notation: I think it's more important and much easier readeable to make good rules about how you handle things and using a strong-typed language. For example, if you access a member of a class, you should never have to worry about the type or if you can write to it: if it compiles, it should work. That's why we have stuff like properties and multiple declarations (overloading) in the first place. And I agree it's best to use names with a length determined by their scope and the amount of local vars you're using. So, 'i' is fine for a loop, 'c' to count stuff, but if you need 'j', 'k', 'l', 'x', 'y' and 'z' as well, you should use more descriptive names overall.

Btw, about optimizations: if you debug a Delphi 5 application and you have a loop that isn't used to index something and counts from 1 to 5, you will notice it actually counts from 4 to 0 instead. And it uses agressive indexing as well, so many local constructs are replaced with indexed conditions. I assumed VS does the same, although I never checked.

 

[Rodéric]

I wonder how you handle inline functions in your code
For a single line function I write this:

class foo
{
protected:
	unsigned int m_uiName;

public:
	inline foo() : m_uiName( 0 ) {}
	inline ~foo() {}

	inline unsigned int getName() const { return m_uiName; }
}

However I'm not sure what to do with multi line (2-3) inlined functions...
I tend to inline only dead simple functions, but I wonder if there's any magic rule to that...
Ideas, suggestions ?

That leads me to another point, I tend to put vars of my SUPER class in protected rather private by default, what do you guys do ?
(Well except when I know I don't want something to be seen by any CHILD class obviously.)

[edit]
Also how do you handle public/private/protected, do you have one "block" for each, or many of each based some reason, and in which order to you put them into the code (public first since "Joe" will be interested only in that) ?

 

[Frank]

Q: In C#, what would you prefer?

byte b = 123;
b = (byte) Convert.ToInt32(b) >> 1;

or:

byte b = 123;
b = b / 2;

?

 

[Nick]

I wonder how you handle inline functions in your code...

Personally I write them the 'Java' way:

class Foo
{
public:
    Foo();
    ~Foo();

    unsigned int getName() const
    {
        return name;
    }

private:
    unsigned int name;
}

Note that the inline keyword is not required when actually inlining the function in the class declaration. I also believe inline is ignored for constructors (leaving it completely to the compiler how they get optimized), but I'd have to check that...

However I'm not sure what to do with multi line (2-3) inlined functions...
I tend to inline only dead simple functions, but I wonder if there's any magic rule to that...
Ideas, suggestions?

When inline funcitons make class declarations harder to read because they are quite lengthy (which is often the case for template classes), I typically structure my header file like this:

namespace XXX
{
    class Argument;

    class Foo
    {
    public:
        // ...
        
        inline int computeSomething(const Argument &a);
    };
}

#include "Argument.hpp"

namespace XXX
{
    int Foo::computeSomething(const Argument &a)
    {
        // ...
    }
}

Note that this is actually just the .cpp file appended to the .hpp file. Using forward declarations whenever possible and including headers in the middle solves a lot of cross dependency problems.

That leads me to another point, I tend to put vars of my SUPER class in protected rather private by default, what do you guys do?

I always make them private first. Only when it becomes clear later in the design that I really need to access the elements in a derived class I'll make them protected. But even in these cases it's best to use protected accessor functions and leave the actual data private. This way the accessor function can do some required transformation or enforce that the data should be read-only. For example if you change the type or semantic of the data you can do the correct transformation in the accessor function of the base class and the derived classes usually don't have to know about it. This can prevent some nasty bugs.

Also how do you handle public/private/protected, do you have one "block" for each, or many of each based some reason, and in which order to you put them into the code (public first since "Joe" will be interested only in that) ?

I typically put the public block first, then protected, then private.