2

So here's a puzzle for some C++ developers here.

I have a custom String class I'm working with that's supposed to teach me how to get comfortable with classes and a few other C++ features and this class has a value property it uses to keep track of its primitive value (which is a std::string).

But here's a problem, I have define multiple methods for creating an object of this class. 2 of which I understand perfectly:

String stringA = "Hello, World!";
String stringB = String("Hello, World!");

But the last one is a bit tricky as it should be able to work with an infinite set of arguments (theoretically).

String stringC = String("Hello,", ' ', "World!");

But when I access the primitive values of these strings, the last one seems to bug out and return (to the best of my debugging efforts) nothing, garbled text or just part of the arguments given.

stringA.valueOf(); // -> Hello, World!
stringB.valueOf(); // -> Hello, World!
stringC.valueOf(); // -> Hello,

Now, this has been a challenge I've really struggled with especially as a JavaScript developer diving into C++. I know the two languages are fundamentally different, but I figure that logically there should be some features of the former that can somewhat be applied to the latter (such as variadic arguments).

For anyone who can help with this and explain to me what I'm missing (or why my code sucks), you're an absolute champ. Great going for you.

#include <iostream>

/* Let's assume there's a `stringify` function that converts any value to `std::string` */
class String {
    private:
        std::string value;

    public:
        template <typename data>
        String(data arg) { this -> value += arg; }

        template <typename data, typename... argumentsData>
        String(data arg, argumentsData... args) {
            this -> value += arg;
            String(args...);
        }

        template <typename data>
        String operator =(data arg) { return String(this -> value = arg); }

        std::string valueOf() const { return this -> value; }
};

int main() {
    String stringA = "Hello, World!";
    String stringB = String("Hello, World!");
    String stringC = String("Hello,", ' ', "World!");

    std::cout << "String A: " << stringA.valueOf() << std::endl;
    std::cout << "String B: " << stringB.valueOf() << std::endl;
    std::cout << "String C: " << stringC.valueOf() << std::endl;

    return 0;
}
Justin
  • 24,288
  • 12
  • 92
  • 142
Lapys
  • 936
  • 2
  • 11
  • 27
  • 3
    `operator=` should update `this` and return reference to self, not return a new string – M.M Oct 24 '18 at 21:03
  • 1
    What do you think the `String(args...);` in the constructor does? What does `int(4);` do? Also, since a newly-constructed `std::string` is always empty, why use `this -> value += ...` when you know `this -> value` was just constructed and so is definitely empty? – David Schwartz Oct 24 '18 at 21:05
  • 3
    I'd recommend you to start with [a good C++ book](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). `stringB` and `stringC` don't use assignment operator here. And as noted by M.M, your assignment operator is not really doing what one would expect it to do. – Yksisarvinen Oct 24 '18 at 21:05

1 Answers1

5

In your constructor

template <typename data, typename... argumentsData>
String(data arg, argumentsData... args) {
    this -> value += arg;
    String(args...);
}

This line: String(args...); creates and discards a temporary String, which doesn't affect the state of the original object.

Instead of trying to solve this with recursion, I recommend using fold expressions:

template <typename ...P>
String(const P &... params) // Passing by const reference is purely an optimization.
{
    ((value += params) , ...);
}
HolyBlackCat
  • 78,603
  • 9
  • 131
  • 207
  • 1
    I would also suggest using forwarding reference and `std::forward` for the args – M.M Oct 24 '18 at 21:04
  • 1
    @M.M `std::string::operator+=` isn't going to move the passed object, so I assume you suggest it simply as a better code style? – HolyBlackCat Oct 24 '18 at 21:08
  • @HolyBlackCat. As a complete noob at C++. I'll be honest and show that I got an error: `binary expression in operand of fold-expression` on the line `(value += params, ...);`. Please how do I prevent this from occuring? – Lapys Oct 24 '18 at 21:11
  • @Lapys Eh, sleepy me forgot to add in the second pair of `()`. Try this: `((value += params) , ...);`. – HolyBlackCat Oct 24 '18 at 21:12
  • 1
    You should be able to simplify the the constructor to `String(P&&... params) : value((stringify(params) += ...)) {}`. Toy example here: http://coliru.stacked-crooked.com/a/86b69c2d487283cb – NathanOliver Oct 24 '18 at 21:17
  • 1
    @HolyBlackCat well it would avoid the local copy `arg` being created (I'm referring to the instances of `data arg` as a parameter) – M.M Oct 24 '18 at 21:17
  • 1
    @M.M The snippet with `data arg` is copy-pasted straight from OPs code, unchanged, to provide a better context. Passing by value is definitely not a good thing here, I agree; that's why I used a const reference in the suggested solution. (In this case it should be no different performance-wise from a forwarding reference.) – HolyBlackCat Oct 24 '18 at 21:24