Sign in to follow this  

Using non-inline function definitions good style?

This topic is 3871 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Recommended Posts

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.

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
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
}

Share this post


Link to post
Share on other sites
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).

Share this post


Link to post
Share on other sites
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.h
class Node
{
Group* parent;
};

//Group.h
class 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.h
class 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.h
class 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.

Share this post


Link to post
Share on other sites
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).

Share this post


Link to post
Share on other sites
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()
{
...
}


Share this post


Link to post
Share on other sites
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).

Share this post


Link to post
Share on other sites
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.

Share this post


Link to post
Share on other sites
Quote:
Original post by VWarriorIs 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.
It's considered fine style if you're not writing professional production code.

As a general rule of thumb, put all of your function definitions out of line in a separate source file. This reduces physical coupling between translaton units and helps reduce the highest cost associated with professional production quality software (that of maintenance).

After running performance analysis, some function definitions can be moved back in line into the headers. Inlining functions can have profound performance benefits, but remember the cost (TANSTAAFL).

One exception is templates: template functions must always be inlined.

Best practice is to have the function inline definition outside of the class definition. This allows the class deinition to be concise and unmodified by moving the function definitions in or out of line.

For an excellent very in-depth treatment of this topic, I would recommend picking up a copy of Lakos's book.

Share this post


Link to post
Share on other sites
Quote:
Original post by Julian90
(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).


But, if I'm not mistaken, aren't inlines dependent on whether they are defined in the class declaration and not on whether they are in the header or source file? As this MSDN definition of inline class member functions states:

Quote:
A function defined in the body of a class declaration is an inline function.
from http://msdn2.microsoft.com/en-us/library/bw1hbe6y(VS.80).aspx

Share this post


Link to post
Share on other sites
Stylistic concerns aside:

Whether or not a function can be inlined typically depends on its definition being available in the same compilation unit. Roughly speaking, this means that the function's code needs to be available in the file after all preprocessor stuff has been run.

Visual C++'s Link Time Code Generation functionality shatters this restriction by delaying the inlining decision to the linker. The linker has access to all functions regardless of where they're located, so it's not a problem.

How much do you want to rely on compiler features? If you'll only ever use VC, you can happily place functions where they fit and the compiler will sort things out for you. But maybe you're cross-platform, and can't rely on that. In that case, you may want to implement in the header (which could mean writing inside the class definition, or outside the class definition but still in the header).

Share this post


Link to post
Share on other sites
There seems to be some confusion over the inline status of templates here. The rule for templates is that the definition of a function has to be visible to the compiler at the point of instantiation (unless you're using the export keyword, which hardly any compilers support). That's orthogonal to the question of whether a function is inlined or not. The inline keyword is just a suggestion to the compiler that the function is a good candidate to be inlined at the call site rather than handled as a function call and according to the C++ standard that means the definition should be visible at the call site, though some compilers like VC are able to inline functions whose definitions are not visible at the call site through link time code generation.

What this generally means is that template functions (and member functions of class templates) must be defined in a header so that the definitions are visible when they are instantiated in any given translation unit (which usually corresponds to a single .cpp file). Template functions defined in a header are not necessarily inlined though - a complex template function is likely to be handled as a regular function call and it's up to the linker to eliminate any resulting code duplication in the final executable.

You can apply the inline keyword to a template function as a suggestion that the function should actually be inlined but standalone template functions and class template member functions defined in the header but outside the class definition where they don't get an implicit inline applied are not considered candidates for inlining according to the standard - again VC and other compilers may be more agressive than the standard requires and attempt to inline functions regardless of whether the inline keyword is used or implied.

To get back to the original question, many coding standards (including the one we follow at work) say you should avoid function definitions within a class definition and that inline functions should be declared in the header (or in an .inl file included in the header) but outside the class definition because they tend to clutter up the class definition. Our coding standard makes an exception for single line function definitions such as typical accessors since they are so short they do not really clutter the class definition and moving them outside is arguably less clear since you have to jump to another location to see that the function is just a simple accessor.

Share this post


Link to post
Share on other sites
Another reason why you may not want to define your declarations inline is that you can't hide the code away from prying eyes. It takes away the notion of a "black box"... if you wish to release your code as prebuilt static libraries to a third party.

Share this post


Link to post
Share on other sites
As an aside, something that will increase your performance by a greater factor will be to return the Point_t by const reference, like so:

const Point_t& getPosition() { return m_position; }

Share this post


Link to post
Share on other sites

This topic is 3871 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

If you intended to correct an error in the post then please contact us.

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this