returning a string from a function

Question

I wanted to write a function that'll be cross platform (win32 & linux), and return a string representation of the datetime [hh:mm:ss dd-mm-yyyy].

Knowing that I just want to use the returned string as a temporary in a stream fashion as below:

std::cout << DateTime() << std::endl;

I considered writing a function with the following prototype

const char* DateTime();

If you return a character array, you must delete it once you're done. But I just want a temporary, I don't want to have to worry about de-allocating the string.

So I've written a function that just returns an std::string:

#include <ctime>
#include <string>
#include <sstream>

std::string DateTime()
{
    using namespace std;

    stringstream ss;
    string sValue;
    time_t t = time(0);
    struct tm * now = localtime(&t);

    ss << now->tm_hour << ":";
    ss << now->tm_min << ":";
    ss << now->tm_sec << " ";
    ss << now->tm_mday + 1 << " ";
    ss << now->tm_mon + 1 << " ";
    ss << now->tm_year + 1900;

    sValue = ss.str();

    return sValue;
}

I realize that I'm returning a copy of the stack variable in DateTime. This is inefficient in that we create the string on the DateTime stack, populate it, then return a copy and destroy the copy on the stack.

Has the c++11 move-semantics revolution done anything to resolve this inefficiency - can I improve upon this?

1) NRVO makes this a complete non-issue. 2) If NRVO didn't kick in for whatever reason, then yes, the return value would be moved rather than copied. — ildjarn
– ildjarn, Commented Jun 29, 2012 at 22:45
@lapin: named return value optimization. Instead of copying a value (perhaps multiple times) to get from the called function back to the caller, the compiler passes the function a hidden pointer to the location where it will end up being assigned, and the function uses that location for its local variable. — Jerry Coffin
– Jerry Coffin, Commented Jun 29, 2012 at 22:49
like other have said RVO is guaranteed if you are using C++11, however I wouldn't worry because the string stream will be the bottle neck here. — 111111
– 111111, Commented Jun 29, 2012 at 23:39
@111111 : RVO is never guaranteed in the standard (98, 03, or 11), merely permitted. It's up to the implementation to make guarantees regarding this particular optimization. — ildjarn
– ildjarn, Commented Jun 30, 2012 at 1:09

Howard Hinnant · Accepted Answer · 2012-06-30 01:12:32Z

6

lapin, your code is fine C++11 code. In C++98/03 your code will probably be efficient due to compiler optimizations, but those optimizations aren't guaranteed. In C++11, those same optimizations will probably still make your return free, but just in case they don't, your string will be moved instead of copied.

So return by value guilt-free! :-)

Minor nit:

It is best practice to declare your values at the point of first use, instead of at the top of a block:

string sValue = ss.str();
return sValue;

Or perhaps even:

return ss.str();

But this is just a minor nit. Your code is fine and efficient.

answered Jun 30, 2012 at 1:12

Howard Hinnant

225k55 gold badges493 silver badges624 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

fishfood Over a year ago

I started the (bad?) habit of declaring my variables before my code when I was writing far too many MSSQL stored procedures a few years ago. I've heard your point before on declaring vars at the point of first use for efficiency, and I agree with it, but I still find myself declaring before the code :)

Nevin · Accepted Answer · 2012-06-30 08:42:42Z

Another way to do this is to make it a function object with a stream inserter, as in:

struct DateTime()
{
    friend std::ostream& operator<<(std::ostream& os, DateTime)
    {
        time_t t = time(0);
        struct tm * now = localtime(&t);

        os << now->tm_hour << ":";
        os << now->tm_min << ":";
        os << now->tm_sec << " ";
        os << now->tm_mday + 1 << " ";
        os << now->tm_mon + 1 << " ";
        os << now->tm_year + 1900;

        return os;
    }

    // Could be converted to a static method,
    //  since DateTime has no internal state
    std::string str() const
    {
        // the following 3 lines can be replaced by
        //  return boost::lexical_cast<std::string>(*this);
        std::ostringstream ss;
        ss << *this;
        return ss.str();
    }

    operator std::string() const
    { return str(); }
};

Simon · Accepted Answer · 2012-06-30 01:16:01Z

-1

Ok, I know this is not thread safe and all that and I'll probably be downvoted to hell end back, but I've seen the following in a library that I'm using (CERN's ROOT):

const char * myfunc(){

  static std::string mystr;

  /*populate mystr */

  return mystr.c_str();
}

This only works if you know that no one will ever be so stupid as to hold on to the pointer.

This is a way of having a temporary that will not leak no matter what.

answered Jun 30, 2012 at 1:16

Simon

1,0212 gold badges11 silver badges14 bronze badges

4 Comments

Howard Hinnant Over a year ago

That was fast DeadMG! :-) The problem with this answer is that it invokes undefined behavior. By the time client sends the pointer to cout, the local mystr has already deleted the character array to which that pointer points to. It might work, and I have no doubt Simon tested before posting. But it also might not work. In a multithreaded environment the same application might sometimes work and sometimes not.

ildjarn Over a year ago

@Howard : mystr is static, so there's no destruction of it here; I think DeadMG's nit is that this isn't thread-safe.

Howard Hinnant Over a year ago

@ildjarn: So it is! Thanks for the correction. I completely overlooked that. What's the convention here? Delete my comment because it might confuse people, or leave it up so they can read this one too?

ildjarn Over a year ago

@Howard : Your call; it might confuse people, or it might be clarifying for other readers that make the same mistake you did when reading the code. :-]

Pete Fordham · Accepted Answer · 2012-06-30 02:15:10Z

-1

In a world without RVO/NRVO this should avoid the copy construction in a pre C++11 standard library. In a post C++11 library with move constructor for string it still avoids the move constructor being called; it's probably a trivially small difference but still the OP was asking how to do better.

(And yes I agree the inheriting from string is ugly but it does work.)

#include <ctime>
#include <string>
#include <sstream>
#include <iostream>

using namespace std;

class DateString : public string {

public:
DateString() : string()     {

    stringstream ss;
    time_t t = time(0);
    struct tm * now = localtime(&t);

    ss << now->tm_hour << ":";
    ss << now->tm_min << ":";
    ss << now->tm_sec << " ";
    ss << now->tm_mday + 1 << " ";
    ss << now->tm_mon + 1 << " ";
    ss << now->tm_year + 1900;

    append(ss.str());

}
};

int main()
{
    cout << DateString() << endl;
    return 0;
}

edited Jun 30, 2012 at 2:15

answered Jun 29, 2012 at 23:51

Pete Fordham

2,34316 silver badges25 bronze badges

1 Comment

ildjarn Over a year ago

1) Inheriting from std::string is just (morally) wrong. 2) The code the OP already has will also avoid copy construction even without NRVO.

Collectives™ on Stack Overflow

returning a string from a function

4 Answers 4

1 Comment

Comments

4 Comments

1 Comment

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

4 Answers 4

1 Comment

Comments

4 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Linked

Related