Is there a non-repetitive way to allow the programmer to choose between copy and move semantics for member initialization?

Question

I want to be able to initialize each field of a class either using move semantics or copy semantics. The constructors will all use essentially the same code for construction, like this:

LogRecord::LogRecord(const Logger &logger, LogLevel level, const std::wstring &message)
    : level(level), logger_name(logger.GetName()), message(message), sequence_number(LogRecord::record_count++), source_class_name(), source_method_name(), time(std::chrono::system_clock::now()) {
}
LogRecord::LogRecord(const Logger &logger, LogLevel level, std::wstring &&message)
    : level(level), logger_name(logger.GetName()), message(message), sequence_number(LogRecord::record_count++), source_class_name(), source_method_name(), time(std::chrono::system_clock::now()) {
}
LogRecord::LogRecord(const Logger &logger, LogLevel level, const std::wstring &message, const std::wstring &source_class_name, const std::wstring &source_method_name)
    : level(level), logger_name(logger.GetName()), message(message), sequence_number(LogRecord::record_count++), source_class_name(source_class_name), source_method_name(source_method_name), time(std::chrono::system_clock::now()) {
}
LogRecord::LogRecord(const Logger &logger, LogLevel level, std::wstring &&message, const std::wstring &source_class_name, const std::wstring &source_method_name)
    : level(level), logger_name(logger.GetName()), message(message), sequence_number(LogRecord::record_count++), source_class_name(source_class_name), source_method_name(source_method_name), time(std::chrono::system_clock::now()) {
}
LogRecord::LogRecord(const Logger &logger, LogLevel level, const std::wstring &message, std::wstring &&source_class_name, const std::wstring &source_method_name)
    : level(level), logger_name(logger.GetName()), message(message), sequence_number(LogRecord::record_count++), source_class_name(source_class_name), source_method_name(source_method_name), time(std::chrono::system_clock::now()) {
}
LogRecord::LogRecord(const Logger &logger, LogLevel level, std::wstring &&message, std::wstring &&source_class_name, const std::wstring &source_method_name)
    : level(level), logger_name(logger.GetName()), message(message), sequence_number(LogRecord::record_count++), source_class_name(source_class_name), source_method_name(source_method_name), time(std::chrono::system_clock::now()) {
}

etc.

Is there a better way to go about this than simply declaring a constructor for each possible combination, like this?

class LogRecord {
public:
    LogRecord(const Logger &logger, LogLevel level, const std::wstring &message);
    LogRecord(const Logger &logger, LogLevel level, std::wstring &&message);
    LogRecord(const Logger &logger, LogLevel level, const std::wstring &message, const std::wstring &source_class_name, const std::wstring &source_method_name);
    LogRecord(const Logger &logger, LogLevel level, std::wstring &&message, const std::wstring &source_class_name, const std::wstring &source_method_name);
    LogRecord(const Logger &logger, LogLevel level, const std::wstring &message, std::wstring &&source_class_name, const std::wstring &source_method_name);
    LogRecord(const Logger &logger, LogLevel level, std::wstring &&message, std::wstring &&source_class_name, const std::wstring &source_method_name);
    LogRecord(const Logger &logger, LogLevel level, const std::wstring &message, const std::wstring &source_class_name, std::wstring &&source_method_name);
    LogRecord(const Logger &logger, LogLevel level, std::wstring &&message, const std::wstring &source_class_name, std::wstring &&source_method_name);
    LogRecord(const Logger &logger, LogLevel level, const std::wstring &message, std::wstring &&source_class_name, std::wstring &&source_method_name);
    LogRecord(const Logger &logger, LogLevel level, std::wstring &&message, std::wstring &&source_class_name, std::wstring &&source_method_name);
    ...
private:
    std::wstring message, source_class_name, source_method_name;
    ...
};

Here is a simplified form to make it a bit easier to read. Object is the class with the members, and Member is the typename of the members. The Member type has both a copy constructor and a move constructor defined.

Basically, my question was how I could do the following with less code duplication:

class Object {
public:
    Object(const Member &x, const Member &y, const Member &z) : x(x), y(y), z(z) {}
    Object(Member &&x, const Member &y, const Member &z) : x(x), y(y), z(z) {}
    Object(const Member &x, Member &&y, const Member &z) : x(x), y(y), z(z) {}
    Object(Member &&x, Member &&y, const Member &z) : x(x), y(y), z(z) {}
    Object(const Member &x, const Member &y, Member &&z) : x(x), y(y), z(z) {}
    Object(Member &&x, const Member &y, Member &&z) : x(x), y(y), z(z) {}
    Object(const Member &x, Member &&y, Member &&z) : x(x), y(y), z(z) {}
    Object(Member &&x, Member &&y, Member &&z) : x(x), y(y), z(z) {}
private:
    Member x, y, z;
}

can' t follow your code, but from the question it looks like you want perfect fowarding http://stackoverflow.com/questions/3582001/advantages-of-using-forward — bolov, Apr 16 '15 at 14:10
@bolov What if you do not want this to be a templated function? — sjdowling, Apr 16 '15 at 14:16
@bolov: If you tell me what about my code makes it hard to follow, I can try to improve it. That said, that link you posted does seem relevant. — RPFeltz, Apr 16 '15 at 14:17
@RPFeltz Mostly I can't follow the code because I am tired. But a few things you could make the code more easily readable: If 2 overloads are enough to illustrate your point, then use only those 2 overloads in the post (you might actually need all, don't know, still didn't read your code). Then you must use the 80 characters limit per line. Horizontal scrolling when reviewing code is... lets say... not enjoyable — bolov, Apr 16 '15 at 14:20

Praetorian · Accepted Answer · 2015-04-16T14:37:11.503

2

I wouldn't bother with all those overloads. Always take the std::wstring arguments by value and std::move them in the mem-initializer. Then you only need 3 constructor definitions. The caveat is that you incur an extra move construction in the cases where you're being passed an rvalue, but you can most likely live with that.

LogRecord(const Logger &logger, LogLevel level, std::wstring message)
    : level(level), logger_name(logger.GetName()), message(std::move(message)), ...
    {}

Note that the move construction might actually be O(n) for small values of n due to small string optimization.

Another option is perfect forwarding as mentioned in the comments. You could do something like

template<typename Message>
LogRecord(const Logger &logger, LogLevel level, Message&& message)
    : level(level), logger_name(logger.GetName()), message(std::forward<Message>(message)), ...
    {}

Maybe add static_asserts to print better error messages that Message is, or convertible to, std::wstring.

edited Apr 16 '15 at 14:37

answered Apr 16 '15 at 14:19

Praetorian

106,671
19
240
328

So if I were to pass a `const std::wstring&` to a constructor that accepts only `std::wstring&&`, would it create a temporary copy, then use move semantics to move that copy's data into the member, then destroy the temporary copy? (referring to the first part of your answer) – RPFeltz Apr 16 '15 at 14:24
@RPFeltz Basically except the constructor only takes a `std::wstring` not a `std::wstring&&` – sjdowling Apr 16 '15 at 14:25
@RPFeltz That wouldn't compile. My suggestion is that the parameter type should be `std::wstring`, not `std::wstring&&`. Added an example. – Praetorian Apr 16 '15 at 14:25
@Praetorian: Oh, sorry, I misread. I must be a bit tired, since when I saw `std::wstring` and `std::move`, I automatically read that as `std::wstring&&`. – RPFeltz Apr 16 '15 at 14:28
The first part doesn't exactly answer the question although you do have a point that the optimization might not matter a lot for short strings (which I will probably mostly be using anyway), but the second part does look like it does what I asked for, so I'll accept the answer nonetheless. – RPFeltz Apr 16 '15 at 14:37
I would add a very strong warning to this answer that passing by value and moving out of it is only advisable for constructors. [This answer](http://stackoverflow.com/questions/26261007/why-is-value-taking-setter-member-functions-not-recommended-in-herb-sutters-cpp) covers why it is not a good idea for setter functions. – sjdowling Apr 16 '15 at 14:41
Could you demonstrate how I should use static_assert in this context to print a better error message? – RPFeltz Apr 16 '15 at 14:47
Put `static_assert(is_convertible::value, "LogRecord requires wstring");` in the body. (uses [type traits](http://en.cppreference.com/w/cpp/types/is_convertible) header) – Thomas B. Apr 16 '15 at 16:24
@RPFeltz [Here's an example](http://coliru.stacked-crooked.com/a/3e933cfc4882fbbb). Unfortunately, the static assertion message is buried amongst other errors due to the invalid call to the `wstring` constructor in the mem-initializer. – Praetorian Apr 16 '15 at 19:03
@RPFeltz I somehow missed your earlier comment about the first part not being an answer to your question. I'd argue that is the correct answer, if you're going to save a copy of an argument the default way to do that should be to take the argument by value and `move` it. Avoid that only if you know the type is expensive to move, or if you absolutely must squeeze every ounce of performance from that code. – Praetorian Apr 18 '15 at 01:15

Is there a non-repetitive way to allow the programmer to choose between copy and move semantics for member initialization?

1 Answers1