15

Assume we have a simple getter method in a class that returns a const reference to a std::string member:

const std::string& getString() const noexcept { return someString; }

With the advent of std::string_view in C++17, I wonder whether it has any advantages of writing this instead:

const std::string_view getString() const noexcept { return someString; }

Does one method have advantages/disadvantages over the other? Clearly (correct me if I'm wrong) both solutions will definitely be better than this:

const char* getString() const noexcept { return someString.c_str(); }

I've seen this related question, but I'm asking for something slightly different.

andreee
  • 3,626
  • 16
  • 33

1 Answers1

20

Yes, you should write:

const std::string& getString() const noexcept { return someString; }

Instead of (note: not const, because never return const values):

std::string_view getString() const noexcept { return someString; }

The reason is - you already have a string. So it's not like you have to pay anything extra to get a string out of it. And string has one notable semantic difference to an arbitrary string_view: it's null-terminated by guarantee. We know this. Maybe some downstream user needs to rely on that information. If they need null-termination (e.g. they need to pass to some C API that requires it) and you give a string_view, they have to make a string out of it themselves. You save nothing, but potentially make downstream users do more work.

If, however, you had a vector<char> instead... then I would suggest to return a span<char const> or the equivalent thereof. Since there is no semantic difference and you're just providing a view.

There also the separate argument of what:

auto x = obj.getString();

should do. This either takes a copy of the string (expensive, but safe) or effectively a reference to it (cheap, but potentially dangling). But it doesn't entirely look like a reference, it looks like a value. This is a broad issue with reference-semantic types in general (things like reference_wrapper, string_view, span, tuple<T&...>, optional<T&> if it existed, etc.).

I don't have an answer for this case, but it's something to be aware of.

Barry
  • 247,587
  • 26
  • 487
  • 819
  • 1
    Regarding the second part: I think in the `const std::string&` case lifetime extension would kick in, which is probably something you loose when using `string_view`s... would this probably be another argument for preferring a const reference instead of a string view? – andreee Jun 14 '19 at 15:48
  • I've seen way too many bugs caused by assuming std::string is null terminated. Unless using c_str() I'd recommend *never assuming*. Sure, it happens to be guaranteed but when changes are made to the code you want the compiler to catch errors, not happily give out pointers to std::vector where someone assumed *v.end() is a legal pointer to a null terminator. – Zan Lynx Jun 14 '19 at 15:52
  • 1
    @andreee lifetime extension will only kick in if your getter returns a prvalue `std::string getString()`. You'll have a dangling reference doing `auto& str = make_obj().getString()` – Guillaume Racicot Jun 14 '19 at 15:57
  • 2
    @ZanLynx What do you mean? `std::string` [guarantees](https://en.cppreference.com/w/cpp/string/basic_string/operator_at) null-termination on basically [every](https://en.cppreference.com/w/cpp/string/basic_string/data) [kind](https://en.cppreference.com/w/cpp/string/basic_string/c_str) of access since C++11... – Max Langhof Jun 14 '19 at 16:01
  • @MaxLanghof All of the bugs happen when you refactor the code to use something *other* than std::string. At that point all of the null termination assumptions *bite you in bad places*. So if it isn't a c_str call don't assume. – Zan Lynx Jun 14 '19 at 16:35
  • to prevent dangling you can have an rvalue overload of getString which moves out the internal string and a `const&` overload for the normal case – Fabio Fracassi Jun 14 '19 at 18:30
  • 1
    @FabioFracassi So the rvalue overload returns a `string` and the lvalue one returns a `string_view`? Dunno how I feel about that. (Also, hi Fabio!) – Barry Jun 14 '19 at 19:53
  • Hi @Barry , You can, but I would probably return `string` and `string const&`, as the string is already there. – Fabio Fracassi Jun 15 '19 at 07:40
  • By returning `const string&`, you are locking yourself into an implementation where you have a dedicated string on hand. Whereas `string_view` is more abstract and allows more freedom within the implementation. In most code you can just flip the return type as needed, but for a more serious contract, I'd recommend `string_view`. It's also very near-free performance wise compared to `string_view` (returning 2 ptr-size fields instead of 1). – VoidStar Jul 23 '20 at 07:59