Best Performance for Getter & Setter

Question

Recently i did some performance tests using getter and setter methods. I'm still not sure which is the best method to use them in c++. (there is no difference using small types like int etc.)

string i;

string GetTest()
{
    return i;
}
void SetTest(string i)
{
    this->i = i; //copy
}

This would be the way, how I would use it in java/C#. No reference/pointer and very slow in c++. For strings, vectors and other "big" types this is very slow and bad if it gets called frequently

string i;
const string& GetTest()
{
    return i;
}
void SetTest(const string& i)
{
    this->i = i; //copy
}

At the moment I'm using this version above, it's much faster than using no references for the parameter and return value and with const I make sure, it won't be changed.

string* i;

string& GetTest()
{
    return *i;
}
void SetTest(string& i)
{
    this->i = &i;
}

This version, using a pointer is faster than just using references, but I think it's harder to read, and if you want to work with the value inside the class, you have to work with the pointer.

Beside using references / pointers it might also improve the performance a little bit by inlining frequently called getter & setters, since they are small functions.

inline GetTest(){....}
inline SetTest(){....}

But as already mentioned, I'm still not sure which would be the best way in c++ to use them, or if there are other ways. It would be good to eliminate these ambiguities.

Edit:

Coming back to this after nearly 4 years, here is my advice I would give now for modern C++: (more helpful than any text)

https://www.youtube.com/watch?v=xnqTKD8uD64 51:00

I recommend to watch the full video, Herb Sutter such an amazing talker, in general follow CppCon.

Answer 1

[I copied this first paragraph exactly from my previous answer at Simple C++ getter/setters ] Generally using accessors/mutators at all is a design smell that your class public interface is incomplete. Typically speaking you want a useful public interface that provides meaningful functionality rather than simply get/set (which is just one or two steps better than we were in C with structs and functions). Every time you want to write a mutator, and many times you want to write an accessor first just take a step back and ask yourself "do I really need this?".

Now if you really want to have getters/setters for specific attributes: Either of the first two versions are correct although the second might perform better in some circumstances. The getters should be const (const string& GetTest() const) so they can be called on const objects.

Your third version with the pointer is pretty dangerous. While the first two have very clear ownership semantics, the third one does not. The short snippet you've shown, with no cleanup, implies that the caller to the set function must keep the string alive for the duration of your class (no ownership transfer). Just avoid that construction completely.

Finally if you're just learning C++ consider one or more of the books at the C++ book list The Definitive C++ Book Guide and List

EDIT: If you're trying to improve the performance of your code the best approach is to compiler with optimization and then profile and see where the hot spots are.

Answer 2

If you are restricted to C++98/C++03, your second code is indeed the optimal solution. The third code is error prone and unidiomatic, and if fixing that by using pointers or, better, smart pointers, as arguments it's still not ideal (and you'll also have to deal with memory management for the string).

If you can use C++11 or C++14, the optimal setter implementation for types that indirectly hold their data and implement move semantics (like std::string) is

void SetTest(std::string a_i) // by value!
{
  i = std::move(a_i); // move assignment
}

The reason why this is optimal is that for lvalue arguments, it will cost about as much as the reference implementation (because the std::move prevents copying of the actual data), but for rvalue arguments (for example the return value of another function, or when explicitly using std::move) the argument is move constructed as well, thus in this case avoiding all copies and being as efficient as your third solution, without its problems.

Note that for types of large size (like structs with many members), already a shallow copy will be costly, and therefore your second version is still optimal for such types even in C++11. For such types an additional rvalue version will not improve performance unless the class additionally manages allocated resources.

Of course, most of the time the correct solution will be to have no getters/setters at all.

Answer 3

These 3 versions you listed in your question do 3 completely different things, that's why they run at different speeds. The "right" way to do string getters and setters in C++ is this:

std::string _test;
const std::string& GetTest() const
{
    return _test; 
}
void SetTest(const std::string& test)
{
    this->_test = test; //copy
}
//optional, not necessary
void SetTest(std::string&& test)
{
    this->_test = std::move(test);
}

Despite what some other contributors say, getters and setters are not necessarily bad class design. They are often preferable to direct access to member variables. However, excessive use of Getters/Setters (or direct access to member variables) is a code smell.

Your first version is solid and correct (except that the Getter isn't const, but it requires the compiler to unnecessarily copy and destroy the string in various situations:

string GetTest()
{
    return i;
}
void SetTest(string i)
{
    this->i = i; //copy
}
...
// this is slower than the "right" way, because it requires creating a copy of the string and destroying it:
bool isEmpty = something.GetTest().size() != 0; // the "right" way does not create a copy here.
// this is slower than the "right way, because it requires creating a copy of the string and destroying it:
std::string myString("I am your father, Luke.");
something.SetTest(myString); // this copies "myString", then calls SetTest with the copy, then destroys the copy

The third version you suggested should be avoided at all cost because it's a brittle design that is guaranteed to lead to unexpected bugs:

string* i;
string& GetTest()
{
    return *i;
}
void SetTest(string& i)
{
    this->i = &i;
}

// this modifies the string after the setter
std::string* myString = new std::string("I am your father, Luke.");
someting.SetTest(*myString);
myString->assign("Oranges");
// now this is true: someting.GetTest() == "Oranges"
delete myString;
// now this is undefined behaviour: someting.GetTest() == "Oranges"

The "right" way to do the third version if you really really really need java/c#-like behavior is the following :

std::shared_ptr<const std::string> _test;
const std::shared_ptr<const std::string>& GetTest() const
{
    return _test; 
}
void SetTest(const std::shared_ptr<const std::string>& test)
{
    this->_test = test;
}
void SetTest(std::shared_ptr<const std::string>&& test)
{
    this->_test = std::move(test);
}

Answer 4

If you store pointers to data in your getter and setter you are going to have major issues with ownership and object lifetimes. Jave and C# have their own ways of dealing with this. In C++ you'll need to use some variety of smart pointer (shared_ptr probably in this situation, but the best guide to this will be to test the actual performance you get).

Answer 5

With most C++ compilers, the small get/set methods will be inlined no matter what you do to them. If you define the implementation in the header file, then they will definitely be inlined!

The only exception is if they are built into a shared library (which is tricky to inline into a caller's code) but even then, you can compile with link-time inlining in modern compilers and then will be inlined there too!

Even with complex types, there's a thing called "return value optimisation" (or RVO) where your return types are compiled to use the same storage as the type used by the caller - ie the entire copy is optimised away.

The trick is to allow the compiler to do its thing by keeping your code simple. Its possible to test your code for performance, but then I think you'll be tweaking performance in other areas anyway.

PS. getters and setters... they're a bad design. Don't encourage them.