Search code examples
c++code-readability

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


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.


Solution

  • A safer implementation would be:

    #define FOO_PROTOTYPES(FOO_VIRTUAL_IMPL)\
    virtual void doSomething() FOO_VIRTUAL_IMPL;\
    virtual void doSomethingElse() FOO_VIRTUAL_IMPL;
    
    class Foo
    {
    public:
        virtual ~Foo() {};
        FOO_PROTOTYPES( = 0 );
    };
    
    class FooImpl : public Foo
    {
    public:
        virtual ~FooImpl() {};
        FOO_PROTOTYPES( override );
    };
    

    Everything is in one header and avoids accidental use of the FOO_VIRTUAL_IMPL value defined in one header in another one.

    However if your class has enough methods to make such constructs worthwhile its probably time to refactor your class into smaller classes.