6

suppose I have a header foo.h like this:

#ifndef FOO_H
#define FOO_H
#include <string>
#include "non_standard_class.h"

std::string foo(MyClass a);
...
#endif

and the implementation foo.cpp will be

#include <vector>

#include "foo.h"

std::string foo(MyClass a)
{
   std::vector<int> x;
   MyClass b;
   ...
}

is it a good pratice to re-include <string> and non_standard_class.h in foo.cpp ? The point is: if I read foo.cpp how can I understand where does MyClass come from? I need to look at foo.h but it will be more difficult.

Ruggero Turra
  • 16,929
  • 16
  • 85
  • 141

3 Answers3

8

The practice I follow:

  • foo.h should directly include all headers its source code needs (i.e. #include <string> if there is std::string in foo.h)
  • foo.cpp should directly include foo.h plus all other headers its own source code needs, except for those already directly included by foo.h

Another formulation of this practice: in foo.*, include only those headers the source code needs. If a header is needed by only foo.cpp (but not by foo.h), then include it from foo.cpp, otherwise include it from foo.h.

So, in your case, I wouldn't #include <string> in foo.cpp, because it's already included by foo.h.

Let's assume that string does an #include <vector>, and foo.h contains std::vector. In this case, I'd #include <vector> in foo.h, because its source code needs it – and it doesn't matter that string already includes it.

pts
  • 80,836
  • 20
  • 110
  • 183
2

Personally I follow the (questionable) minimal inclusion practice: I only include what's needed for compilation to work.

People telling to get a better compiler / machine quite make me laugh, in a way, the project I am working on daily is compiled in about 8 minutes (from scratch) on a quad-core (was about 1 hour before I scrapped down dependencies) and I find it too damn long. I certainly don't want any change to cause the whole thing to recompile, and therefore I am wary of any include.

During development, I always feel like compilation time is wasted time. A bit might allow a break sure, but when you want to test something now, it's only frustrating to wait for the damn thing to finally be ready (especially if it just blows in your hand when testing and you realize you'll have to modify it again and re-compile again).

Including things twice seems like a good way to forget to remove the include from the source when it's no longer need by the header (hurray for forward declaration), I don't see the point.

Matthieu M.
  • 287,565
  • 48
  • 449
  • 722
1

I would say it is, because otherwise your foo.cpp is relying on a detail of foo.h that may change. Obviously this is more of a problem with other .h files that aren't under your control, but I still do it for consistency sake.

Paul Tomblin
  • 179,021
  • 58
  • 319
  • 408
  • -1, because this introduces extra maintenance work, and `foo.cpp` and `foo.h` are hardly going to be changed independently -- they're corresponding files – Stuart Golodetz Dec 11 '10 at 12:58
  • And in addition to that, it may introduce more work for the compiler if applied consistently across a project. – Stuart Golodetz Dec 11 '10 at 12:59
  • If you think including a file twice introduces significant extra work for the maintainers and the compiler, you need better maintainers *and* a better compiler. – Paul Tomblin Dec 11 '10 at 13:03
  • 1
    @Paul: I didn't say "significant" extra work for the compiler -- I don't think that. There is potentially a bit of work involved however, and there's no need for it. I do think that including every header you include in a .h in the .cpp as well across the entire project can lead to unnecessary extra maintenance work. Maybe not massive amounts of extra maintenance work, but why add any at all when it's unnecessary? – Stuart Golodetz Dec 11 '10 at 13:25
  • 1
    On the obverse side of that, if the .h file no longer requires string.h but the .cpp still requires it, *not* including it in the .cpp file is now extra work for the maintainers. You can't just baldly assert that including the file where you need it is going to cause extra work. – Paul Tomblin Dec 11 '10 at 13:27
  • 2
    @Paul: but if you somehow `foo.h` does not require `string` anylonger, then don't you think that somehow you will need to work on `foo.cpp` anyway ? If so, then it's time to add the header. – Matthieu M. Dec 11 '10 at 13:32
  • But by @Stuart's standards, that constitutes "extra maintenance work". If it was already in there, that horrible burdensome work would already be done for you. – Paul Tomblin Dec 11 '10 at 13:37
  • @Paul: Ok, I agree that the maintenance aspect of it is more arguable :) – Stuart Golodetz Dec 11 '10 at 13:38
  • 1
    I guess my main objection here is that I don't see the point of including the same header twice if it can be avoided -- doing it as a deliberate act of policy "in case the header changes later" seems silly. If the maintenance involved is really as minor as you suggest, why anticipate it up-front? – Stuart Golodetz Dec 11 '10 at 13:40