Using non-inline function definitions good style?

Started by
14 comments, last by _goat 16 years, 11 months ago
From my understanding, an example is, in a class you'd specify and define a function in the header file. Example: foo.h

// accessor method
Point_t getPosition()   {return m_position};

Is that considered good style? Personally, I don't like it, but I can see the point of doing it because the function is one small simple line of code.
Advertisement
If the code is small, it is better to inline it, rather then have
unneccessary overhead of pushing/popping the stack, and call.

I personally dont think it would impact newer systems performance much though.
Of course, this depends on what is being pushed and popped.

Some compiliers allow inline routines within the class declaration,
while being able to define it outside its class. This isnt
standard though (That I know of)

With the code you provided, you have to find a good balance between
good coding practices (encapsulation) and performance. Coinsidering
it is a very small obvious routine, I recommend inline it.
that is an inline function (even though you didnt specify it to be inline, it's implicitly inline if its inside a class definition).

Personally, I like to move all my inlines into a seperate header file.

So i have blah.cpp:
#include "blah.h"//implmenet CBlah here...


blah.h:
class CBlah { inline DoSomeBlah();};#include "blah.hpp"


and blah.hpp:
inline CBlah::DoSomeBlah(){//do something}
Arguments Against:
1) The compiler doesnt have to inline it
2) It increases compile times
3) It's in compatable with idioms such as pimpl
4) It could be considered inconsistent since the rest of the functions are probally in the source file.

Arguments For:
1) The compiler doesnt have to but it probaly will inline it
2) Its required for templates
3) Its not inconsistent if you have a strict guide that you follow when deciding weather to inline it
4) It reduces the amount of duplicate information. (functions signitures etc) which can get out of sync.

Quote:Some compiliers allow inline routines within the class declaration,
while being able to define it outside its declaration. This isnt
standard though (That I know of)


Its standard but you have to specify inline on the definition (but not the decleration).
Ok, thanks guys! I didn't really think about it terms of stack overhead. So think I think now i'm more in favor of it.
I used to always do it for trivial Get/Set functions and the like, but now I never do it, as it can lead to linking hell. Consider the case of a simple graph structure. You have a base class Node, and a derived class Group. The Node class contains a pointer to a Group object indicating its parent, and the Group class contains an array of pointers to child objects of type Node. Here you have an otherwise trivial case of two classes referencing each other, or a cyclical dependency.
//Node.hclass Node{	Group* parent;};//Group.hclass Group :public Node{	std::vector<Node*> children;};



This is easily handled with a properly structured header system. The Node class must be fully defined before the Group class is fully defined, as a base class must be defined in order to inherit from it. The Node class also references the Group class, but it only holds a pointer to an object of type Group. In this case, the Group class only needs to be declared, not fully defined, before the body of the Node class is defined, like so:
//Node.hclass Group;class Node{	Group* parent;};//Group.h#include "Node.h"class Group :public Node{	std::vector<Node*> children;};


As soon as you throw in some functions which are inlined within the body of the Node class however, if any of those inlined functions dereference the pointer to the Group class, the Group class now has to be fully defined before that function can be defined. As the function is contained within the Node class body, that also means the Node class cannot be fully defined until the Group class is fully defined, leaving us with an unsolveable dependency loop.
//Node.hclass Group;class Node{public:	int FooBar()	{		return parent->SomeFunc(); //Boom! The compiler doesn't know what					   //the members of the Group class are yet.	}	Group* parent;};//Group.h#include "Node.h"class Group :public Node{public:	int SomeFunc()	{		return children.count();	}	std::vector<Node*> children;};

You now have no choice but to separate the function definition from the class body.

If you inline functions using this technique as a matter of habit, you will run into this problem repeatedly when working on larger projects. Dependency issues when linking a project are some of the must frustrating and messy problems to deal with. You can avoid many hours of tedium and frustration by conforming to a few simple rules, one of which is that you do not define functions within the body of a class.


All that said, there is always an exception to the rule. You can get away with functions being defined within the body of a class if the class is trivial, or a basic structure as opposed to a class (eg, a couple of members and corresponing Get/Set functions). Never ever do it within a complex class however, or a class which may be inherited from, or you're just inviting problems. If a simple class grows into a not-so-simple class, you should also separate out the bodies of any existing functions which may have been defined within the class.

I have defined functions within the body of a class on occasion where I thought it was appropriate, but I still wouldn't recommend doing it. Like many often frowned upon language features however, they are useful in specific cases. Treat them like explosives. If you don't know what you're doing, stay far away, and even if you do, handle with care.
I always write functions inlined in the class definition. Then, I may choose to move them to the implementation file when refactoring, if they are too long (more than half a screen) or too complex (multidirectional control).

I have never had any problems with circular dependency (of which I only have minute amounts anyway).
I write member functions inline as long as they follow these rules:
1) The function definition is between 1 to ~3 statements long
2) It doesn't call another function
3) ...and something else that I can't remember right now, sorry.

Julian90 -
Quote:2) Its required for templates


Are you sure? I've always written my templates with both inline and non-inline functions. But if you are going to put them non-inline you have to put the template keyword before the function, like this:

template <typename Type>class CBlah{  ...public:  int size();};template <typename Type>int CBlah<Type>::size(){  ...}
The problem with putting the function implementation in the class declaration is that it bloats the declaration, making it harder to read. The advantage is that it is easier and more convenient to maintain.

So, there are tradeoffs and no correct answer.

Generally when people implement functions in the declaration, they only do it if the function is very short (I might do it if it is a single line, for example).
John BoltonLocomotive Games (THQ)Current Project: Destroy All Humans (Wii). IN STORES NOW!
Quote:Original post by Switch0025
Julian90 -
Quote:2) Its required for templates


Are you sure? I've always written my templates with both inline and non-inline functions. But if you are going to put them non-inline you have to put the template keyword before the function, like this:

*** Source Snippet Removed ***


Now try putting the template function in the source file (which is what I was refering to, I consider it inline if the function decleration is in the header even if its not in the class decleration).

Unless your compiler supports "export" (only EDG) then it wont work, you'll get linker errors when you try and compile it.

This topic is closed to new replies.

Advertisement