0
\$\begingroup\$

I'm designing a generic interface for a MessageEntry class for a .h file.

I'm looking for guidance on form and terminology (would I call this an interface/template/generic class?):

#ifndef cppmq_message_types_h
#define cppmq_message_types_h

template <class T>
struct MessageEntry {
 public:
  MessageEntry<T>() {}
  MessageEntry<T>(T data) : data(data) {
    creation_time =
        std::chrono::system_clock::to_time_t(std::chrono::system_clock::now());
  }
  time_t CreationTime() { return creation_time; }

 private:
  T data;
  time_t creation_time;
};

#endif`
\$\endgroup\$

closed as unclear what you're asking by Barry, rolfl, vnp, 200_success Sep 16 '15 at 21:58

Please clarify your specific problem or add additional details to highlight exactly what you need. As it's currently written, it’s hard to tell exactly what you're asking. See the How to Ask page for help clarifying this question. If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. \$\endgroup\$ – Barry Sep 16 '15 at 21:00
  • \$\begingroup\$ What is this code supposed to do? Please explain, and edit the title accordingly — see How to Ask. \$\endgroup\$ – 200_success Sep 16 '15 at 21:58
  • \$\begingroup\$ I have restored your original (broken) code. Please see What you may and may not do after receiving answers \$\endgroup\$ – rolfl Sep 17 '15 at 15:54
2
\$\begingroup\$

There are several problems with this code, starting from the fact that it is syntactically invalid. Getting past that, you have no access to your data, which makes it somewhat pointless, and the only way to construct a MessageEntry is to preconstruct a T, then copy it into the MessageEntry constructor, then copy it again into data. That can be unnecessarily expensive, and then you're not actually timestamping when you constructed the original object - you're timestamping two copies later.

You definitely want to be able to support in-place construction and you definitely want easy access to the data. Fortunately, you can solve both problems in one go with just simple inheritance:

template <typename T, typename Clock=std::chrono::system_clock>
struct MessageEntry : T
{
    using T::T;
    decltype(Clock::now()) creation_time = Clock::now();
};

That's... it. I added a second template argument in case your users want a different clock (e.g. std::chrono::steady_clock). I intentionally left creation_time non-const so as to make the class copyable. If you don't need this to be copyable, then I would write it as:

template <typename T, typename Clock=std::chrono::system_clock>
struct MessageEntry : T
{
    using T::T;
    const decltype(Clock::now()) creation_time = Clock::now();

    MessageEntry(MessageEntry const& ) = delete;
    MessageEntry& operator=(MessageEntry const& ) = delete;
};

It might be worth renaming the class to something that adds clarity to its intent. Perhaps WithCreationTime.

Lastly, I would prefer #pragma once just to be able to write less code for include guards, and you are missing your #include for <chrono>.

Full solution:

#pragma once

#include <chrono>

template <typename T,
          typename Clock = std::chrono::system_clock>
struct WithCreationTime : T
{
    using T::T;
    decltype(Clock::now()) creation_time = Clock::now();
};

Example usage:

WithCreationTime<std::vector<int>> v{1, 2, 3, 4};
std::cout << v.size() << '\n'; // 4
std::cout << v.creation_time << '\n'; // whatever now() is
\$\endgroup\$
  • \$\begingroup\$ Usually I don't like to inherit from the template argument, since it kind of mix horizontal and vertical design. Of course, there are exceptions (CRTP being the most famous); in this case the design is fine, since the added feature of the derived class/struct is universal and orthogonal to the template argument. The alternative would be to store a T member, and use variadic template in the constructor (something similar to what emplace does), but the design may be heavier (though someone may prefer it). \$\endgroup\$ – bartgol Sep 16 '15 at 21:38