To inline or not to inline

Question

I've been writing a few classes lately; and I was wondering whether it's bad practice, bad for performance, breaks encapsulation or whether there's anything else inherently bad with actually defining some of the smaller member functions inside a header (I did try Google!). Here's an example I have of a header I've written with a lot of this:

class Scheduler {
public:
    typedef std::list<BSubsystem*> SubsystemList;

    // Make sure the pointer to entityManager is zero on init
    // so that we can check if one has been attached in Tick()
    Scheduler() : entityManager(0) { }

    // Attaches a manager to the scheduler - used by Tick()
    void AttachEntityManager( EntityManager &em )
        { entityManager = &em; }

    // Detaches the entityManager from a scheduler.
    void DetachEntityManager()
        { entityManager = 0; }

    // Adds a subsystem to the scheduler; executed on Tick()
    void AddSubsystem( BSubsystem* s )
        { subsystemList.push_back(s); }

    // Removes the subsystem of a type given
    void RemoveSubsystem( const SubsystemTypeID& );

    // Executes all subsystems
    void Tick();

    // Destroys subsystems that are in subsystemList
    virtual ~Scheduler();
private:
    // Holds a list of all subsystems
    SubsystemList subsystemList;

    // Holds the entity manager (if attached)
    EntityManager *entityManager;
};

So, is there anything that's really wrong with inlining functions like this, or is it acceptable?

(Also, I'm not sure if this'd be more suited towards the 'code review' site)

Answer 1

Inlining increases coupling, and increases "noise" in the class definition, making the class harder to read and understand. As a general rule, inlining should be considered as an optimization measure, and only used when the profiler says it's necessary.

There are a few exceptions: I'll always inline the virtual destructor of an abstract base class if all of the other functions are pure virtual; it seems silly to have a separate source file just for an empty destructor, and if all of the other functions are pure virtual, and there are no data members, the destructor isn't going to change without something else changing. And I'll occasionally provide inlined constructors for "structures"—classes in which all data members are public, and there are no other functions. I'm also less rigorous about avoiding inline in classes which are defined in a source file, rather than a header—the coupling issues obviously don't apply in that case.