C++: Is it good practice to make getters and setters inline?

Question

Hi,

The title says it all.

public:
     inline int GetValue() const {
          return m_nValue;
     }
     inline void SetValue(int nNewValue) {
          this -> m_nValue = nNewValue;
     }

On Learn C++, they said it would run faster. So, I thought it would be great.
But maybe, there are some negative points to it.

Answer 1

The code will compile slightly longer and you lose encapsulation. Everything depends on the size of the project and its nature. In most cases it's OK to make them inline if they do not have any complex logic.

BTW, you may skip inline if you implement directly in class definition.

Answer 2

I don't inline anything until a profiler has specifically told me that not inlining is resulting in a performance problem.

The C++ compiler is very smart and will almost certainly automatically inline such simple function like this for you. And typically it's smarter than you are and will do a much better job at determining what should or should not be inlined.

I would avoid thinking about what to or not to inline and focus on the solution. Adding the inline keyword later (which is not a guarantee of inline BTW) is very easy to do and potential places can be found readily with a profiler.

Answer 3

Negative points:

1) The compiler is free to ignore you.

2) Any change to these functions requires recompilation of all clients.

3) A good many compilers will inline non-inlined functions anyway when appropriate.

Answer 4

If you write them inside the definition, they are inlined by default.

Answer 5

This is Bad practice in public API's Any change to these functions requires recompilation of all clients.

In general having getters and setters is showing poor abstraction, don't do it. If you are constantly going to the raw data in another class then you likely need to re arrange your classes, instead consider how you wish to manipulate the data within a class and provide appropriate methods for doing so.

Answer 6

I'd say that you don't need to bother with that. Read the FAQ section about inlining.

Answer 7

By putting the code in the header, you are exposing your internal class workings. Clients may see this and make assumptions on how your class works. This can make it more difficult to change your class later without breaking client code.

Answer 8

No need, start trusting the compilers, at least for such optimizations!
"But not always"

Answer 9

I don't do this to be honest, but I'm interested also to know if this is better.

Answer 10

I'd also like to add that unless you're performing millions of gets/sets per frame, it's pretty much irrelevant whether these are inlined or not. It's honestly not worth losing sleep over.

Also, keep in mind that just because you put the word 'inline' in front of your declaration+definition, doesn't mean the compiler will inline your code. It's uses various heuristics to work out whether it makes sense, which is often the classic trade off of speed vs size. There is however the brute force '__forceinline' keyword, at lease in VC++ (I'm not sure what it is in GCC), which stomps on the compilers fancy heuristics. I really don't recommend it at all, and besides once you port to a different architecture, it'll likely be incorrect.

Try to put all function definitions in the implementation file, and leave the pure declarations for the headers (unless of course you're template metaprogramming (STL/BOOST/etc), in which case, pretty much everything is in the headers ;))

One of the classic places people like to inline (at least in video games, which is where I'm from), is in math headers. Cross/dot products, vector lengths, matrix clearing, etc are often placed in the header, which I just think is unnecessary. 9/10 it makes no difference to performance, and if you ever need to do a tight loop, such as transforming a large vector array by some matrix, you're probably better off manually doing the math inline, or even better coding it in platform-specific assembler.

Oh, and another point, if you feel you really need a class to be more data than code, consider using good old struct, which doesn't bring the OO baggage of abstraction with it, that's what it's there for. :)

Sorry, didn't mean to go on so much, but I just think it helps to consider real world use cases, and not get too hung up on pedantic compiler settings (trust me, I've been there ;))

Good luck.

Shane

Answer 11

I gotta say, I don't have the strong aversion to this practice that others on this thread seem to have. I agree that the performance gain from inlining is negligible in all but the most heavily used of cases. (And yes, I have encountered such cases in practice.) Where I do this kind of inlining, I do it for convenience, and generally just for one-liners like this. In most of my use cases, the need to avoid recompilation on the client side if I ever change them just isn't that strong.

Yes, you can drop the inline, as it's implied by the placement of the implementation.

Also, I'm a little surprised at the vehemence against accessors. You can hardly sneeze at a class in any OO language without blowing a few down, and they are after all a valid technique to abstract implementation from interface, so it's a bit masochistic to claim them as bad OO practice. It is good advice not to write accessors indiscriminately, but I also advise you not to get carried away in the zeal to eradicate them.

Take that, purists. :-)