How to design a simple std::string-to-boost::posix_time::ptime parsing library

Question

I recently found the need to parse a std::string to a boost::posix_time::ptime. The question of how to accomplish this task is answered well here: How to parse date/time from string?.

I know it only requires a few lines of code (basically the istringstream declaration, locale/time_input_facet imbuance, and extraction to ptime), but I wanted to wrap that functionality in a convenience function and make it available as a C++ library for my various programs that work with ptimes.

If possible, I'd like to get some feedback from C++ gurus on my design, just so I can know if I've done anything obviously wrong or if there are better approaches out there. Particularly, I'm not very familiar with how locales and time_input_facets work, so I hope I don't have a memory leak or anything.

Here it is:

namespace mydt {

    void imbueDTFormat(std::istringstream& iss, const std::string& format );
    void parse(const std::string& input, const std::string& format, boost::posix_time::ptime& out ); // (1)
    void parse(const std::string& input, std::istringstream& iss, boost::posix_time::ptime& out ); // (2)
    void parse(std::istringstream& iss, boost::posix_time::ptime& out ); // (3)

    void imbueDTFormat(std::istringstream& iss, const std::string& format ) {
        iss.imbue(std::locale(std::locale::classic(), new boost::posix_time::time_input_facet(format) )); // see <http://en.cppreference.com/w/cpp/locale/locale/locale>: "Overload 7 is typically called with its second argument, f, obtained directly from a new-expression: the locale is responsible for calling the matching delete from its own destructor."
    } // end imbueDTFormat()

    void parse(const std::string& input, const std::string& format, boost::posix_time::ptime& out ) { // (1)
        static std::istringstream iss;
        imbueDTFormat(iss, format );
        parse(input, iss, out );
    } // end parse()

    void parse(const std::string& input, std::istringstream& iss, boost::posix_time::ptime& out ) { // (2)
        // assumes iss has already been imbued with the desired time_input_facet
        iss.str(input);
        parse(iss, out );
    } // end parse()

    void parse(std::istringstream& iss, boost::posix_time::ptime& out ) { // (3)
        // assumes iss has already been imbued with the desired time_input_facet AND has been initialized with the input str
        iss >> out;
    } // end parse()

} // end namespace mydt

The first function I wrote was parse (1). It provides the easiest interface; just pass the input string, format string, and the ptime OUT var, and the parsing is done. It uses a static istringstream to complete the parsing, so no allocations are necessary for that. But when I looked at the function after I wrote it, I realized that if you have a single format that you repeatedly want to parse from, then it's wasteful to repeatedly allocate a new time_input_facet from it and imbue the same istringstream with it.

So I figured that you could do better by having the calling code create its own (perhaps static) istringstream, imbue the format once, and then repeatedly use that istringstream to parse. Hence I wrote parse (2) for that purpose. So calling code could have one dedicated function for each format, like so:

void parseServerDT(const std::string& input, boost::posix_time::ptime& out );
void parseHostDT(const std::string& input, boost::posix_time::ptime& out );

// in main, or some other code context
boost::posix_time::ptime serverDT; parseServerDT(getServerDTStr(), serverDT );
boost::posix_time::ptime hostDT; parseHostDT(getHostDTStr(), hostDT );

void parseServerDT(const std::string& input, boost::posix_time::ptime& out ) {
    static bool first = true;
    static std::istringstream iss;
    if (first) {
        first = false;
        mydt::imbueDTFormat(iss, SERVERDT_FORMAT );
    } // end if
    mydt::parse(input, iss, out );
} // end parseServerDT()

void parseHostDT(const std::string& input, boost::posix_time::ptime& out ) {
    static bool first = true;
    static std::istringstream iss;
    if (first) {
        first = false;
        mydt::imbueDTFormat(iss, HOSTDT_FORMAT );
    } // end if
    mydt::parse(input, iss, out );
} // end parseHostDT()

I think this design should provide maximum convenience for calling code, and should minimize the memory and performance impact. You could define as many parseXDT() functions as you like (and even create a macro to reduce the duplicate code in those functions.)

Any feedback would be much appreciated. Thanks!

Answer 1

At the very least

1. make the statics threadlocal. With lacking compilers this involves dynamically allocating them (not an issue since it's a one time cost)

2. most importantly, clear the stream error state and contents before re-use

   void parseHostDT(const std::string& input, boost::posix_time::ptime& out ) {
       thread_local std::istringstream* iss = [] {
           first = false;
           mydt::imbueDTFormat(iss, HOSTDT_FORMAT);
       }();
   
       iss->clear();
       iss->str("");
       mydt::parse(input, iss, out);
   }