Is it good practice to make getters and setters inline?

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 to use on getters and setters. But maybe, there are some drawbacks to it?

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.

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

This means that they will be permitted in multiple compilation units (since class definitions themselves typically appear in multiple compilation units), not that they will actually be inlined.

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.

Negative points:

1. The compiler is free to ignore you.

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

3. A good compiler will inline non-inlined functions anyway when it is appropriate.

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

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.

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.

The inline keyword is meaningless in your case

Compiler will inline your function if it can and wants to, regardless of keyword.

The keyword inline affects linking and not inlining. It is a bit confusing, but read up on it.

If the definition is in a different compilation unit (source file after preprocessor, basically) than the call, inlining will only be possible if whole project optimization and link time code generation are enabled. Enabling it greatly increases linking time (since it practically re-compiles everything in the linker), but obviously may improve performance. Not sure if it's on or off by default in GCC and VS.

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

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

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. :-)