Class Putting Self in std::vector

Question

This is a small test to try and add a copy of a containing class into a vector. To me, it looks very ugly. I hate the fact I need to write a constructor based off of a pointer in order to add the class to a vector. Is there a better way to organize this?

#include <string>
#include <sstream>
#include <vector>
#include <iostream>

Support function (not the main point of the question):

template <class T>
std::string to_string(const T& number) {
 if (std::is_arithmetic<T>::value) {
 std::stringstream ss;
 ss << number;
 return ss.str();
 } // else
 return "to_string not implemented"; // I could throw here...
}

Main code:

class Return_Self_Test {
private:
 std::string name;
public:
 Return_Self_Test(const std::string& name_in) : name(name_in) { }
 Return_Self_Test(Return_Self_Test* rst) : name(rst->name) {
 }
 Return_Self_Test(const Return_Self_Test &obj) : name(obj.name) {
 }
 void output() const {
 std::cout << "name: " << name;
 }
 std::vector<Return_Self_Test> get_in_vector() const {
 std::vector<Return_Self_Test> rst;
 rst.emplace_back(Return_Self_Test("new one 1"));
 rst.emplace_back(*this);
 rst.emplace_back(Return_Self_Test("new one 2"));
 return rst;
 }
};
void return_self_test() {
 Return_Self_Test rst_a("Original");
 auto a_vect = rst_a.get_in_vector();
 for (size_t i = 0; i < a_vect.size(); i++) {
 std::cout << to_string(i) + ": ";
 a_vect.at(i).output();
 }
}

return_self_test() gets called from main;

Any comments welcome!

• Ben Steffan: Just to make sure I am understanding what you are trying to achieve here: You want a method that "wraps" an object in an std::vector, right?
• David: @Ben that's right. I also have the extra 'new ones' in there as a test.

Answer 1

#include <iostream>
#include <ostream>
#include <string>
#include <vector>
class Return_Self_Test {
 std::string name;
 public:
 Return_Self_Test(std::string const& name_in) : name(name_in) { }
 std::string const& get_name() const noexcept {
 return name;
 }
 std::vector<Return_Self_Test> get_in_vector() const {
 using std::literals::string_literals::operator""s;
 std::vector<Return_Self_Test> rst;
 rst.emplace_back(Return_Self_Test("new one 1"s));
 rst.emplace_back(*this);
 rst.emplace_back(Return_Self_Test("new one 2"s));
 return rst;
 }
};
std::ostream& operator<<(std::ostream& os, Return_Self_Test const& rst) {
 return os << "Name: " << rst.get_name();
}
void return_self_test() {
 Return_Self_Test rst_a("Original");
 auto a_vect = rst_a.get_in_vector();
 for (size_t i = 0; i < a_vect.size(); i++) {
 std::cout << std::to_string(i) + ": " << a_vect[i] << '\n';
 }
}

This is my suggestion for a slightly more organized solution of your problem. Let's walk this through and see what I did differently from you, and why.

Constructors

C++ is actually reasonably nice when it comes to what special member functions have to be declared by the user and which ones are generated by the compiler. For most simple classes, you actually don't need to define anything at all! Copy constructor, move constructor, copy assignment operator, move assignment operator and destructor are all implicitly defined in your case, so I went ahead and removed your copy constructor definition.

You should also notice that I removed that pointer-to-self constructor you defined, which you also expressed concerns about. I have good news for you: This constructor is completely unnecessary! In fact, your original code does not even use it, and neither does mine.

Where is to_string?

I don't know why you felt compelled to write your own version of this function, but the standard provides something with the exact same functionality in the string header (well, apart from the fact that std::to_string will downright reject anything that is not a number).

Still, let's take some time and talk about your version of this common functionality. Three things come to my mind here:

1. Why do you take number by const reference? You are expecting numeric types, which are all small enough to be efficiently passed as values.
2. Having a normal if on a compile-time type trait is an slightly bad, if you have access to C++17. In that case, this should clearly be if constexpr (...).
3. Regard and handle errors as errors. Don't return some ill-defined dummy value if your function does not succeed. There are many ways to signal an error, from throwing an exception to returning an std::optional. In this special case, however, the error is actually diagnosable at compile time. Although this makes point two useless, check your type at compile time and SFINAE out if it is not a number. You will save yourself a lot of debugging headaches down the line.

Doing I/O Correctly

Maybe you have been wondering: "Hey, where did my output method go?". Probably, you have already realized that I turned it into a non-member operator<<, and I do have good reasons to do so:

Imagine you are working with a huge project with thousands and thousand of classes. Your task is to implement a logging system: Sometimes, things go wrong, and it is your task to collect all the error messages that result from this, order them by certain criteria, and send them to various outputs (e.g. a log file, stdout, a web socket). However, all classes only offer a single output() method, just as Return_Self_Test does, which writes a message to std::cout.

So you spend the next half a year doing the most boring, mind-numbing refactoring work anyone could possibly imagine.

Do you understand what I am getting at here? You are basically loading a gun to shoot yourself in the foot with in a few years when you write code like this. C++ offers a whole lot of flexibility through the whole istream/ostream concept, and you currently use exactly nothing of it.

This is why you should start writing operator<< instead of your own output or write or whatever methods. This makes integrating output routines into an already existing context much more pleasant, as you can just write these values to std::cout as you are used to with all of the basic types. Furthermore, using a general std::ostream instead of std::cout will save you a lot of suffering when you find yourself in a situation where you actually have to write to something that is not the standard output.

• David: Great response. Thank you for your comments.
• David: I have a few questions: Why use a string literal for my constructors? What advantage does that give? Should I think of noexcept like const, as in mark all functions that can't call exceptions with it? I noticed you are missing a return on ostream - I will edit that in. Thank you again for your analysis. Exactly the type of feedback I was looking for.
• Ben Steffan: @David The ""s operator is useful here to get away from implicit conversion of char const* to std::string. Also, this operator can determine the size of the underlying string literal at compile time, saving you a few calls to std::strlen. noexcept is a lot like const, except that it is not as important and is thus disregarded by many programmers. However, noexcept enables some useful optimizations (in particular with things from algorithm) and adds some additional guarantees for yourself and your users.