.h and .cpp

[Daniel]

For some very interesting coding standards referenced by no less
authority than Stroustrup himself:

http://www2.research.att.com/~bs/JSF-AV-rules.pdf

Interesting to see the recommended naming conventions,

Example_class_name

RGB_colors

example_function_name

Underscores don't seem to be that popular though, more likely to be
seen are ExampleClassName, RGBColors or RgbColors, and
exampleFunctionName.

Daniel

 

[Christopher]

  That's a null statement. If a header file contains implementations,
they will be inline by necessity (else you'll get linker errors if the
header is included more than once), so that rule isn't really saying
anything.

I've seen this claim in several of the replies. I made a minimal
example that compiles and links in Visual Studio 2008 SP1 (what they
use at work).

--ClassA.h---
#ifndef CLASSA_H
#define CLASSA_H

#include <iostream>

class A
{
public:

A()
:
m_x(0)
{
std::cout << "Contructor" << std::endl;
}

~A();

int Method1(const int y)
{
m_x = 4 * y;
return m_x;
}

int Method2(const int y);

private:

int m_x;
};

#endif

--ClassA.cpp---

#include "ClassA.h"

A::~A()
{
}

int A::Method2(const int y)
{
m_x = 2 * y;
return m_x;
}


--main.cpp---

#include "ClassA.h"


int main()
{
A a;
std::cout << a.Method1(5) << std::endl;
std::cout << a.Method2(5) << std::endl;
}

--dummy.cpp--
#include "ClassA.h"

void Foo()
{
for(int x = 0; x < 10; ++x)
{
A a;
std::cout << "I constructed an A " << x << "times." <<
std::endl;
}
}

------

ClassA.h contains implementation of the A class. So does ClassA.cpp.
ClassA.h is included in more than one compilation unit: ClassA.cpp,
main.cpp, and dummy.cpp
No linker errors.

Yet, this is an example of what I am talking about that should not be
done. The constructor and Method1 are not inline, and have no business
being implemented in the header. They should be in the .cpp, unless
they were inlined. Keep in mind these are simple for example purposes.

 

[Ian Collins]

I've seen this claim in several of the replies. I made a minimal
example that compiles and links in Visual Studio 2008 SP1 (what they
use at work).

--ClassA.h---
#ifndef CLASSA_H
#define CLASSA_H

#include<iostream>

class A
{
public:

A()
:
m_x(0)
{
std::cout<< "Contructor"<< std::endl;
}

This is an inline function.

 

[Christopher]

Yes no linker errors because Method1 is inline;

/Leigh-

I'm using it. I was not aware it could be inline without explicitly
using the inline keyword.
I just read the faq 9.6 after your response. Learn something new
everyday.

Ok, so I should instead argue that the "best practices" in the FAQ are
followed, which state that the method should be defined outside the
class body, using the inline keywork. It would make the inlining
obvious and keep the spirit of the class defintion telling us what the
class offers instead of how. Furthermore, there should be a clear
advantage in inline the method for it to be inlined. 60,000 line
functions that spawn threads are probably not good canidates for
inlining.

 

[ralph]

Interesting to see the recommended naming conventions,

Example_class_name

RGB_colors

example_function_name

Underscores don't seem to be that popular though, more likely to be
seen are ExampleClassName, RGBColors or RgbColors, and
exampleFunctionName.

Daniel

What you are describing is called Camel Case or Camelback. Most
historians trace its use as a direct migration from using inner
underscores as a delimiter to using uppercase to separate words.
Mostly because it saved space and yet still provides readability.

Underscores were popular in the early days of C and C++, and it is
just that popularity that has led to discouraging their use. So many
libraries and implementations used underscores - collisions were
inevitable.

ANSI now forbids them in some contexts.

However, it might be useful to remember before posting that there are
three subjects that are best avoided in this newsgroup:
1) Favorite Editors,
2) Indent Styles,
3) and Naming Conventions

-ralph <bg>
[*Comments should receive honorable mention.]

 

[Jorgen Grahn]

What you are describing is called Camel Case or Camelback. Most
historians trace its use as a direct migration from using inner
underscores as a delimiter to using uppercase to separate words.
Mostly because it saved space and yet still provides readability.

Underscores were popular in the early days of C and C++, and it is
just that popularity that has led to discouraging their use.

Underscores is still my main style, for function and variable names.
It's also the style for the Linux kernel and many Unix utilities.

I suspect under_score is simply an old Unix convention. Most other
OSes seem to use some CamelCase variant. (Happily, none that I know of
uses ALL_UPPERCASE as the main style.)

So many
libraries and implementations used underscores - collisions were
inevitable.

That doesn't make sense -- you cannot invent a new naming convention
for each library! Prefixes is the way it's done, pre-namespaces at
least.

ANSI now forbids them in some contexts.

However, it might be useful to remember before posting that there are
three subjects that are best avoided in this newsgroup:
1) Favorite Editors,
2) Indent Styles,
3) and Naming Conventions

Yes, but we're ok as long as there's no advocacy involved.

/Jorgen

 

[Jorgen Grahn]

I'm using it. I was not aware it could be inline without explicitly
using the inline keyword.
I just read the faq 9.6 after your response. Learn something new
everyday.

Ok, so I should instead argue that the "best practices" in the FAQ are
followed, which state that the method should be defined outside the
class body, using the inline keywork. It would make the inlining
obvious and keep the spirit of the class defintion telling us what the
class offers instead of how.

Well, the class definition already tells us a lot about the "how" --
the private section. I happily put trivial definitions in the class
itself, if it doesn't /overshadow/ the "this is the interface" view of
the class.

Besides, it's not as if defining the inline functions after the class
doesn't obscure anything else. I frequently have a number of "loose"
functions there which are just as important as the contents of the
class itself. It's a tradeoff, unfortunately.

Furthermore, there should be a clear
advantage in inline the method for it to be inlined. 60,000 line
functions that spawn threads are probably not good canidates for
inlining.

Better example: a function which compiles into a lot of object code,
and is called from many places.

/Jorgen

 

[Nick Keighley]

I cannot visualize this scenario. Can you give a code example in which
any part of class implementation is required to be in the header,
excluding the use of templates?

Use of inclusion guards has always taken care of duplicate symbols
when a header is included in more than one source file and is pretty
standard practice in my experience.

not if it's in two different compilation units. The key thing is "if
the class parts are /defined/ in a header" (emp. mine). In a nutshell
a declaration of an object (this probably applies to non-objects like
functions as well) says "this is the name of something- but doesn't
allocate any space" whilst a definition gives it soemwhere to live.
Naming something more than once is ok. Creating more than one copy of
something is wrong. This is sometimes referred to as The One
Definition Rule.

In this case, what's wrong with putting it in the header? I suppose
until I can visualize your argument for 1b and 2, I have trouble
seeing this as well.

I'd say don't put it in a header if you don'e need to. It's just work.

Whoops, I believe I meant to say variable _declaration_. Definition
and Declaration seem to very confusing terms as they have different
meanings for classes and variables in C++.

What I mean is
int x = 10; should not appear in a .h file.
int x; However, may appear in a .h file, but I'd still argue that
there is probably a better way then using a global.

"global" isn't really a C++ term. ITYM "external". If you are using
them then declare 'em external.

// externals.h
int x;

// externals.c
#include "externals.h"
int x = 10;

// user.c
#include "externals.h"

void f()
{
doThing (x);
}

const int x = 10; Should not appear in a .h file
const int x; May appear in a .h file, but I'd rather see:

static int GetX() const
{
   return 10;

}

that should be inline

because some other global MyClass might make use of x and then I run
into the static intialization fiasco.

really? This surprises me. Are const items of basic types really
subject to SIF? I thought declaring consts in a header was ok.

const double PI = 3.1415926535;

I thought the idea was to be able to replace C's

# define PI 3.1415926535

to have to hide it in a static function seems really icky. I tghink
I'll start using #define again...

or better if only used in one class:

class WhereXIsUsed
{
public:
private:
  const int m_x;

};

or possibly something like

struct SystemConfigurationDefaults
{
   const int m_x;

};

namespaces maybe?

One is a superset of the other.
But again, I think I meant declared instead.

you also need to expalin what a "global" is. The term is not terribly
well defined.

<snip>

 

[Juha Nieminen]

The only non-inline functions you can have in header files are templates.

Template function definitions behave exactly like inline function
definitions (in that they can be included in more than one compilation
unit and, if no literal inlining happens, the linker will merge all the
instances into one).

I have heard rumors that there are some technical differences between
(non-inline) template functions and inline functions, but I really don't
know what those differences might be.

 

[none]

I am beginning to think that might be the case, but if the boss says
"I beleive you, you just have to give some reasoning aside from I said
so." Then I wanted to at least try before joining the "I am looking
for employment" group again.

Well, you should be able to justify the rules in your coding
guideline. Some of the justification may be weak but they should still
be rational. If you can't justify a rule, you may as well remove it
and only kep the ones that can be justified.

A typical rule is naming convention. Why is it better to use
CamelCase rather then under_score_separated_names (or vice-versa).
Taken in isolation, neither is better than the other. However, it can
be clearly demonstrated that a code base that use either randomly is
significantly harder to read than a code base that use a consistent
naming convention.

Many other rule may appear not useful at first sight but when thinking
about long term maintenance, they start becoming useful. Having some
classes defined in cpp means that they are potentially harder to
find. Keeping files shorter mean that it is easier to track changes
and potential side effects, etc.

 

[none]

I've mentioned it before, but IMHO -Wall is far from enough. I default
to

-Wall -Wextra -pedantic -std=c++98 -g -O3

You weakling! "Real Men" use:

-Wall -Wextra -pedantic -std=c++98 -Weffc++ -Wold-style-cast -Werror

 

[none]

Don't know about hpp for templates. It's *.h or *.hpp for everything
if you ask me. If there is only one class ion the file, or if it's
clearly the "main" class in it, then class_name.h is only natural.

Maintainability: if the general rule that each class "class_name" is
defined in class_name.h and implemented in class_name.cpp, navigating
the code is easier than if 7 different classes are defined in file1.h
and randomly implemented in file2.cpp and file3.cpp and some additonal
classes are defined and implemented entirely in file3.cpp

Disagreed. There's classes that will only be used in one translation
unit. What can one gain by making another trivial header?

I don't entirely disgree with you. but in large project, this can be a
valid policy.

Typically, what will happen is devA decides he needs a little helper
class for his big class, he knows he only needs it here in this
specific translation unit so just define and implement it fully inside
big_class.cpp.

Then often, devB, who is implementing another_class, needs some helper
functionality. It's fairly generic functionality so he peeks in the
generic area for a header file that might indicate that this
functionality is already available. He doesn't see anything so he
implements my_helper_class inside another_class.cpp ith functionality
almost to same as that implemented by devA previously.

Then so on for DevC, DevD ...

If you are lucky, maybe DevE actually finds the implemention that
exists in big_class.cpp and figure out that he should reuse it rather
than duplicate the functionality. Unfortunately, for him to reuse it,
he now has to extract first_helper out of big_class.cpp, create
first_helper.h and first_helper.cpp.

If instead DevA wrote first_helper in its own files, any future reuse
would have been much easier.

In a previous life, I once searched through the code base of a large-ish
C project and found something like 4-5 different implementation of a
XML-quoting function, all of them defined and implemented locally.

Then there's unit testing: if your class is only defined inside a a
cpp file, it's rather difficult to unit test it.

More important in my opinion:

For every header file the following should compile without error or
warning:

#include "the_header.h"
int main() { return 0; }

if you need to preceed #include "the_header.h" by anything, your
header file is not self contained and should be fixed.

Yannick

 

[Juha Nieminen]

-Wall -Wextra -pedantic -std=c++98 -Weffc++ -Wold-style-cast -Werror

Turning a bunch of warning flags and then making them errors is an
extremely dubious practice. Why? Because your program might compile
with one version of a compiler, but not with another, solely because
of the "-Werror" and without there being any real mistake in the program.
(It might also not compile with a different compiler which supports the
same flags. For example I think Intel's compiler supports most of the
same flags as gcc.)

There's a relatively popular open source program whose main developers
had the odd principle that any official build must be built with every
single warning flag that they could find on the gcc documentation, plus
-Werror. This had the adverse side-effect that completely inconsequential
warnings, such as the compiler not being able to inline a function marked
as "inline" (and, IIRC, even if it was unable to inline a *template*
function) would cause the compilation to fail, even though there were no
mistakes whatsoever in the code.

This completely idiotic principle caused that the program had to be,
basically, fine-tuned for a specific version of gcc. A newer or older
version of gcc with different optimization strategies could well decide
not to inline one of those functions, causing the program not to compile.
(And yes, this indeed was the case in practice.)

I don't know if they still have this idiotic principle.

 

[none]

Turning a bunch of warning flags and then making them errors is an
extremely dubious practice. Why? Because your program might compile
with one version of a compiler, but not with another, solely because
of the "-Werror" and without there being any real mistake in the program.
(It might also not compile with a different compiler which supports the
same flags. For example I think Intel's compiler supports most of the
same flags as gcc.)

There's a relatively popular open source program whose main developers
had the odd principle that any official build must be built with every
single warning flag that they could find on the gcc documentation, plus
-Werror. This had the adverse side-effect that completely inconsequential
warnings, such as the compiler not being able to inline a function marked
as "inline" (and, IIRC, even if it was unable to inline a *template*
function) would cause the compilation to fail, even though there were no
mistakes whatsoever in the code.

This completely idiotic principle caused that the program had to be,
basically, fine-tuned for a specific version of gcc. A newer or older
version of gcc with different optimization strategies could well decide
not to inline one of those functions, causing the program not to compile.
(And yes, this indeed was the case in practice.)

I don't know if they still have this idiotic principle.

So seriously:

I don't use -Werror however I use a policy that normally, all warnings
should be silenced. Warnings oftens indicate real bugs, if the code
compile with hundreds of warning, peoples stop reading them.

-Werror is certainly unacceptable for code that is distributed in
source format and expected to build in various environment. However,
in a closed source environment where that build environment changes
either never or very rarely, this could be used as a way to enforce
silencing warning. I prefer a more diplomatic approach.

I generally get on with -Wextra. -Weffc++ is very hard to follow all
of the time in a large project. In particular third party headers
tend to raise a lot of warnings. I've never managed to get on with
-pedantic on a large project either. It just tend to generate too
many warnings and warnings loose value due to noise.

Yannick

 

[Juha Nieminen]

I don't use -Werror however I use a policy that normally, all warnings
should be silenced. Warnings oftens indicate real bugs, if the code
compile with hundreds of warning, peoples stop reading them.

That's certainly true. My point was, however, that not all warning flags
are indicative of (potential) errors in the code (nor even bad practice).
Some of them are purely informative (such as the warning that an inline
function couldn't be inlined).

(Turn on enough warning flags and even the standard libraries will start
issuing warnings.)

In other words, you should choose your warning flags carefully.

 

[Jorgen Grahn]

Turning a bunch of warning flags and then making them errors is an
extremely dubious practice. Why? Because your program might compile
with one version of a compiler, but not with another, solely because
of the "-Werror" and without there being any real mistake in the program.
(It might also not compile with a different compiler which supports the
same flags. For example I think Intel's compiler supports most of the
same flags as gcc.)

There's a relatively popular open source program whose main developers
had the odd principle that any official build must be built with every
single warning flag that they could find on the gcc documentation, plus
-Werror. This had the adverse side-effect that completely inconsequential
warnings, such as the compiler not being able to inline a function marked
as "inline" (and, IIRC, even if it was unable to inline a *template*
function) would cause the compilation to fail, even though there were no
mistakes whatsoever in the code.

We do this in a project, and I agree: -Werror sucks. I've lost many
weeks[1] to warnings about unused parameter/functions/variables, and
it also affects code quality negatively since people start writing
strange code to avoid the warnings-become-errors. That we are heavily
#ifdefed doesn't help either.

Two things though:

- In absence of people who feel responsible to keep the number of
serious warnings down, -Werror can save the code. But then you have
a more fundamental problem ...

- In recent gcc releases you can selectively make warnings fatal or
not -- so you can e.g. get warnings for unused functions and fatal
errors for bad printf formatting strings.

/Jorgen

[1] Counting nightly builds which failed due to a harmless warning,
and had to be rescheduled for the next night.

 

[Jorgen Grahn]

.

Many other rule may appear not useful at first sight but when thinking
about long term maintenance, they start becoming useful. Having some
classes defined in cpp means that they are potentially harder to
find.

But that's a *good* thing! Not being able to find irrelevant things is
a good thing.

More to the point, if you're scanning the header files looking for
whatever, you don't want to find things which only concern the inner
workings of one translation unit.

/Jorgen

 

[Rui Maciel]

But that's a good thing! Not being able to find irrelevant things is
a good thing.

More to the point, if you're scanning the header files looking for
whatever, you don't want to find things which only concern the inner
workings of one translation unit.

If each class is declared/defined in separate, dedicated source files then
there is no need to employ any grep voodoo to ignore what is considered
irrelevant when searching for any declaration/definition.


Rui Maciel

 

[Jorgen Grahn]

If each class is declared/defined in separate, dedicated source files then
there is no need to employ any grep voodoo to ignore what is considered
irrelevant when searching for any declaration/definition.

Sorry, I don't understand your reasoning. Can you give an example?
It seems to me that it's when you *do* move implementation details
into the header files that you have to employ "grep voodoo" to
eliminate some of the noise.

I have never tried to do it that way, but it feels like I would end up
with too much global information to keep track of -- things that would
normally be safely hidden away in an anonymous namespace.

/Jorgen