Is it good practice to pass `char []` to a function which accepts `std::string&`

Viewed 754

I am not facing any issues with below code but is it good practice to pass char [] to a function which accepts std::string& as a parameter

 const char* function(std::string& MyString)
 {
    MyString = "Hello World";
    return MyString.c_str();
 }

 int main()
 {
    char MyString[50];

    /*
    *Is it good practice to cast like this?
    *what possible issues i could face because of this casting?
    */
    function((std::string)MyString);                  

    std::cin.get();
    return 0;
 }
3 Answers

This simply won't work because it will require creation of temporary std::string which can not be bound to l-value reference. Even if function took a reference to std::string const creation of temporary would have an impact of performance. So depending on nature of the function it may be a good idea to add an overload that accepts a pointer to a c-string as well. Alternatively, if function is not going to modify the string you can make it accept std::string_view so it can handle both std::string and c-strings.

No, it is bad practice, as the cast has no effect; a std::string can be constructed from a char * with a non-explicit constructor, so you can remove the cast and you'll get exactly the same code (just with an implicit construction instead of an explicit cast).

Now as written, you'll get an error (at least with a non-broken compiler), as you can't pass a temporary object to a non-const lvalue reference. But if you change the function to take a const std::string &, it will work just fine.

Also bad practice is returning the char * you get by calling std::string::c_str() -- this pointer will only be valid as long as the string object is not modified or destroyed -- so the returned pointer will become invalid (dangling) as soon a the temp you passed as an argument was destroyed. If you were to save that returned pointer in a local variable in main and then try to do something with it (like printing it), that would be undefined behavior.

In short passing char[] to function accepting string is common practice (from C). And it is not bad. The explicit cast is not good here. The function also is not good, as it not accept passing char[] ...

Related