Is it okay and/or normal to use #include to put large chunks of repetitive code into a separate file?

Question

I have been dissecting some code and I saw something I haven't seen before, and I was wondering if it is a good/bad practice and if it is normal.

Basically there is a header file with a class definition for a class with a bunch (around 90) of pure virtual functions. There are a lot of these virtual functions, so they are all put into a separate file and then included in the class definition like so:

Foo.h

class Foo
{
public:
    virtual ~Foo() {};
    #define FOO_VIRTUAL_IMPL = 0
    #include "Foo_prototypes.h"
};

Foo_prototypes.h

#if ! defined(FOO_VIRTUAL_IMPL)
# define FOO_VIRTUAL_IMPL
#endif

virtual void doSomething() FOO_VIRTUAL_IMPL;

virtual void doSomethingElse() FOO_VIRTUAL_IMPL;

Also

Is using the define macro like that also common (i.e. allowing the same include file to be used for pure virtual and normal virtual functions)? Is this sort of stuff used often, or just for little hacks to save a little bit of time/effort?

I suppose these things made the code seem less readable to me, but it may just be because I am not used to these tricks and once I get used to them I will be better equipped to read this sort of code.

The code in question is Interactive Brokers' C++ API, if anyone cares to see it in context. Relevant files are: EWrapper.h and TestCppClient.h and EWrapper_prototypes.h.

Answer 1

Wellll, this isn't exactly what I'd call well-styled code. Yes, technically you can include headers in that kind of place, but it's not exactly standard, so I wouldn't recommend it. Just in general, "stay away from the dusty corners of a language". However, if you really want to do that kind of thing, you can. However, not following standard practices can have two effects.

1. Non-standard code is generally harder to read, so less people will be able or willing to contribute.

2. Non-standard code can have more bugs that aren't as noticed, just because it's not standard.

An example of the bugs I mentioned beforehand is the way FOO_VIRTUAL_IMPL works. #define's aren't limited to scope, so this would be visible to all your code. It would be really really easy to #define it in one header, and not define it at all in another header. This would cause all the virtual functions in the second header to be purely virtual, probably not what you intended.

EDIT: Also, as Caleth said, if your class requires that much repetitive code, it would be good to redesign your class entirely.

Answer 2

There are several issues here.

If you have a class with the same pure virtual functions over and over again, it's an interface by defintion. There is absolutely no reason not to wrap things up in a well-defined, named interface. Modern IDE's will support you when implementing those interfaces (so there is no need for macro-magic).

Generally, using macros is bad. We wouldn't accept macro-programming in any code-review. Check out the C++ ISO guideline for more info:

Macros are a major source of bugs. Macros don't obey the usual scope and type rules. Macros ensure that the human reader sees something different from what the compiler sees. Macros complicate tool building.

https://github.com/isocpp/CppCoreGuidelines/blob/master/CppCoreGuidelines.md#Res-macros