C++ pet-peeves

Management / people related pet-peeves

* People who say that "macros are evil" or that require that macros
cannot be used. Use templates where you can. But don't be afraid to
use macros if it makes sense. Andrei Alexandrescu and Matthew Wilson
both use macros in their books and libraries where it makes code
cleaner and easier to read.

* People / rules that make unnecessary requirements. For example:
code must be single entry, single exit.

* Companies that will not provide the tools to help the developers
do the best job possible. (Ex: "Memory leak detection software is too
expensive.", GCC/GDB is free, why do you need an integrated development
enviroment.)

* People who mandate rules just to save source-code space. Ex:
Function names can only be 10 characters long.

* People / companies who refuse to use source control systems.

* Developers who believe that testing departments exist to find all
the bugs and consequently do nothing to develop tests themselves.

* Companies that do not do code reviews

* Development teams where nobody has the authority to make a final
say. (This usually happens on a two-person co-development environment
where neither was designated as the lead developer).

* People / rules that require hungarian notation in C++.

Compiler-Specific pet-peeves

* "#pragma once" instead of header guards (what if you need to use a
non-MS compiler?)

* Using #include <not-a-system-header>
It won't compile on many compilers.
When questioned, the usual answer is "because I think that header is important"

Generally frowned upon C++ practices

* "using someNamespace" in header files

* Classes that are not const-correct

* lack of const (and I mean lack of const in each and every single isntance where you're not required
not to use it).

* Unitialized variables

* throw new T();

* throw "Oops, there was an error";

* Relying on undefined behavior (or compiler-specific behavior for things defined as "undefined behavior" by the standard)

Documentation / comment pet-peeves

* cryptic comments

* incorrect / out-of-date comments

* unnecessary comments:

++i; // increment i

* Missing and/or incomplete documentation.
(It's truely a joy to add something to a program wich you basically have to reverse engineer to figure out where to fit in the new code.)

* Wrong documentation
(Even worse then missing documentation, at least with missing documentation you know you've to figure it out yourself.)

Really nice. There are so many things. The discussion thread could go uncontrolled. Better would have been you compiled these stuff and added into the FAQ list or made an article out of it and then the moderators could hang them on top of the non-visual C++ forum as a sticky thread or something. Or may be some other way round. Thanks from my side for putting up such an exhaustive list. Its for sure going to help me.

Also, could I use my free-mail id to register to the comp.lang.c++.moderated group? How do we get connected to it. I tried once but it needed some configuration with my email account and I was not able to figure it out.

To read comp.lang.c++.moderated, I just use google groups without subscribing:

http://groups-beta.google.com/group/comp.lang.c++.moderated

-------------

It's funny you should post this. I have recently been seeing a lot of these as I work through the code of excellent programmers who do things a little different than me. Most of the pet-peeves I see at work are of the Reliving C variety. One that wasn't mentioned (I don't think) is declaring all the variables at the top of the function, and even worse "initializing" them on a separate line of code just below:void Foo()
{
BOOL bRet;
int nValue;

bRet = FALSE;
nValue = GetCurrentValue();

// ...
}Aarggh!! :D

It takes all my will power not to go through every function and change these things, and even then sometimes I do it anyway. :p

You've got to add TABS vs. SPACES in code! All code should be indented with SPACES! Not everyone uses the same text editor.

;)

Viggy

Here's some of my peeves


Management / people related pet-peeves


*Companies that think something can be done in five minutes when in they don't understand the depth of what they asked for and it takes far longer. "What's taking you so long??!!"

*Companies that think software engineers can develop anything and do not care whether or not it's designed properly.

*Time sheets anyone?

*Software engineers with too much pride and try to cover up their mistakes. "It's not my **** code" mentality.

*Feature creep



Programming Peeves!


*Deep deep inheritance hierarchies.

*"I_can_do_it" functions a.k.a. "Fat Functions".
Functions that have way too much code in them and think they can do it all. It's also unfortunate that these functions tend to be a critical/important point in the software as well.

*Excessive coupling between software components.

*Bad cohesion

*Having to maintain and add features to a poorly written legacy system

*Excessive use of global variables

*Spaghetti code

*Poor/confusing naming conventions

Doing

Code:
char *p = "flop";
char &c = *p;
Instead of

Code:
char* p = "flop";
char& c = *p;

or rather

const char * p = "flop";

and I like the space both sides of the operator.

if (condition) statement;
while (condition) statement;

VS

if (condition)
statement;

while (condition)
statement;

VS

if (condition) {
statement;
}

while (condition) {
statement;
}

vs

if ( condition )
{
statement();
}

while ( condition )
{
statement();
}

My biggest pet peeve is one that I've seen in another team member's code:

unsigned i = 0;

This one makes my blood boil. Is it that hard to type "int"?

No but it is a pain typing unsigned, which is why I like to typedef unsigned int to uint or uint_t or uint32 if you want to specify a 32-bit unsigned int. It also looks tidier if a type has a single name.

I typedef const std::string & to str_cref as well.

When you say a "public header". if you mean the entry point to a library, I never use std::string an the interface to any library because it is non-portable. Your std::string may have a different representation to mine if we are using different versions of STL.

I have a wrapper class portable_basic_string and the typedef to portable_string with char as its type is in its own header. pstr_cref can also be put in its own header in which case you don't need to search around to see what it means.

Hmm, about that one
{
T *p = new T();
// ...
delete p;

}

instead of just:

{
T t;
// ...

}
There may be a reason for preferring the 1st one in some cases. If the object is too large to be placed on the stack... What do you think?

I will publish my "portable_basic_string" sometime but it relies on a shared-pointer. My shared-pointer is based on boost but is slightly different. In boost's shared-pointer it calls d(p) with your deleter, whereas with mine it calls a virtual function destroy() and doesn't pass the pointer at all. The deleter is given the pointer on its construction, and deletes itself at the same time as it deletes the pointer, thus the deleter and the pointer have the same lifetime.

So my deleter for an array of characters looks like this:

template < typename T >
struct self_array_deleter : public deleter_base
{
private:
const T* itsPointer;
public:
void destroy()
{
delete [] itsPointer;
delete this;
}
self_array_deleter( const T* pt ) : itsPointer( pt ) {}
};

The boost equivalent version would be:

template < typename T >
struct array_deleter
{
void operator()( const T* ptr )
{
delete [] ptr;
}
};

With boost you can use shared_array though, which should work here. It defaults to checked_array_deleter() (I think) if you don't specify otherwise.

So it's portable only if you are using the same version of boost. (But as I am not using boost in my situation everyone is forced to use the similar smart pointer I have).

(By the way, you can put down companies who don't use boost as a pet-peeve too. I generally rewrite the bits I want on similar lines to how boost have implemented them...).

As a junior developer this thread is great!

Thanks everyone.

polus quietly creeps out of the thread to change his ways - stop doing the if(TRUE == bResult) .. and int nVar = 0; nVar = GetVar()... :D

actually I wouldn't stop from using int nVar = 0; regardless what comes after it. It creates and maintains the good habbit of initializing all the variables as soon as they are defined. Perhaps, later on you will remove the nVar = GetVar(); but you will forget to add the initialization on the line above.

Yeah, this is a long list. I didn't think that this would fit well into the FAQ section as these are more just what get's people annoyed, not about firm rules.

I was also interested to see if there were other pet-peeves people had that didn't make the list. (And I was kind of hoping to see if any debates would open up about the code layout issues. I actually find these debates quite informative; from some I have actually learned what makes things clearer for the newbie and therefore less prone to maintenance errors.)

:D

Cheers!

Quite a lot of stuff here I agree with (in the whole thread) but, as usual, not everything!

Also, some things are unavoidably subjective. The problem is in separating out the subjective from the objective. You get the same thing with coding standards. I find that, while I have preferences, there are some practices which I dont like but which Im prepared to tolerate and others that I wont tolerate.

Reading through this I see you guys are worried about the portability when using STL containers. Why is this? Are the interfaces they use not specified by the standard? Dont they all support the same set interfaces? Surely how they do their thing is irrelevant as long as the functions/algorithms do what the standard specifies. I thought as long as you used the standard functions of these containers and you made no assumptions about how the STL class in questions inner workings work then your code was portable. Am I wrong then?

For example, if I write a library using MSVC 8.0 that exposes a function "void Foo(const std::string& str);", but someone uses Digital Mars's compiler to interface with it, then the user's application will pass a reference to Digital Mar's implementation of std::string rather than the expected MSVC 8.0 std::string. Any differences of internal layout will lead to undefined behavior.
This is not just the case for STL. You could say the same thing about the FILE structure from stdio.h.

Regards,

Paul McKenzie

Hmm, about that one
{
T *p = new T();
// ...
delete p;

}

instead of just:

{
T t;
// ...

}
There may be a reason for preferring the 1st one in some cases. If the object is too large to be placed on the stack... What do you think?

If the object is too large to be placed on the stack then it should use new/new[] internally to allocate the large array/whatever makes the object so large (just like std::vector - I have yet to come across a scenario where you need to create a vector using new). It is not reasonable to rely on the programmer to use new when creating the object because it might not fit on the stack.

My company's policy is to hide the internals (we're quite protective about the internal engineering IP) and consequently have to think about these things. We've been bitten most often by structure alignment issues because most of our API is C. But if we go to C++, the STL containers will be another thing we'll have to think of.