0

During code review I repeatedly found the following pattern in some other persons code:

void writeFile(std::string_view path)
{
    auto of = std::ofstream(std::string{path});
    ...
}

As far as I understand, using string_view in the functions signature avoids the string from being copied. Is there any point in using string_view in the signature and then converting it to std::string (and thus explicitly making a copy) in the next line? Isn't using a std::string parameter achieving exactly the same?

Simon Fromme
  • 3,104
  • 18
  • 30
  • The answer to this question depends entirely on how this function is being called, and with which types of objects that are being passed here as a parameter, whether `const char *`, `std::string`, or another `std::string_view`. You will need to provide more information in your question. – Sam Varshavchik Feb 08 '21 at 04:23
  • If you're asking specifically about `ofstream`, it has a `const char*` constructor that avoids the need for making a new `std::string` anyway. – chris Feb 08 '21 at 04:24
  • @SamVarshavchik: They are mostly called with an argument previously defined as `auto constexpr TMP_FILE{"/tmp/foo.txt"sv};` – Simon Fromme Feb 08 '21 at 04:26
  • 3
    @chris though that assumes that `path` is a null-terminated string, which is not necessarily the case—that's the whole reason that it requires taking a `std::string` instead of a `std::string_view` in the first place :) – N. Shead Feb 08 '21 at 04:26
  • @N.Shead, Fair enough. It wasn't explicitly mentioned in the question, but I should have made a note of that. (And of course a `zstring_view` isn't a standard option currently.) – chris Feb 08 '21 at 04:30
  • The function should not make assumptions on what may or may not be calling it. It advertises a `std::string_view` interface and should assume nothing more. So, unfortunately, creating the `std::string` is necessary (or similar like a `std::filesystem::path`). – Galik Feb 08 '21 at 04:52
  • If it is any consolation, any function that is going to read from a disk it likely to be relatively time-consuming and therefore the copy will be negligible. – Galik Feb 08 '21 at 04:54
  • And what it the reason for "previously defined" arguments like "`auto constexpr TMP_FILE{"/tmp/foo.txt"sv};`"? What is the program attempting to accomplish by defining string view literals? Unless there is some other, specific, intentional reason for doing that, this accomplishes nothing useful whatsoever and it requires a conversion to `std::string` here. Simply get rid of string views, define them as plain, old-fashioned, `const char *`, or maybe `const char[]`, pass them to this function as `const char *`, and that's going to be the end of it. – Sam Varshavchik Feb 08 '21 at 12:16

0 Answers0