Variable initialization fails to compile with comma-operator

Question

Why does the line indicated ( in main() ) in the following code not compile?

#include <iostream>
#include <string>
#include <map>

template< typename _T > struct Inventory : public std::map< _T, int >
{
    bool getat(int i, _T &t, int &n)
    {
        if ((i < 0) || (i >= (int)this->size())) return false;
        int c=0;
        typename std::map< _T, int >::iterator it = this->begin();

        while ((c < i) && (it != this->end())) { c++; it++; }
        t = (*it).first;
        n = (*it).second;
        return true;
    }
    Inventory &operator=(_T t) { (*this)[t]++; return *this; }
    Inventory &operator,(_T t) { return operator=(t); }
};

int main()
{
    int i=0, quantity;
    std::string item;

    //***Fails to compile:***
    //Inventory< std::string > inv = "a","b","c","a","c","d","e";  

    Inventory< std::string > inv;
    inv = "a","b","c","a","c","d","e";    //but this is fine
    inv = "e","f";

    while (i < (int)inv.size())
    {
        if (inv.getat(i, item, quantity)) 
                std::cout << i << ": " << item << " -> " << quantity << "\n";
        i++;
    }
    return 0;
}

If your compiler supports `#include `you could play with `Inventory inv { "a","b","c","a","c","d","e" }; `. See [here](http://msdn.microsoft.com/en-us/library/hh567368.aspx) for VS2012 (doesn't support). — rubber boots, Oct 10 '12 at 11:17
@rubberboots: I'm using gcc; combining your suggestion with the `Collector`-idea from James Kanze's answer could be useful. — slashmais, Oct 10 '12 at 11:48
@slashmais - you don't need both. If the compilers you'll be using all support initializer lists, that's the way to go. If not, use the `Collector` idea. — Pete Becker, Oct 10 '12 at 14:17

Luchian Grigore · Accepted Answer · 2012-10-10T12:36:39.390

2

That's called copy-initialization. In short, it uses the conversion constructor and then the copy constructor to construct inv, not operator = as you expect.

Inventory< std::string > inv = "a","b","c","a","c","d","e";

is invalid syntax. Something like Inventory< std::string > inv = "a" would first attempt to create a temporary Inventory from "a","b","c","a","c","d","e" and then use that temporary as argument to a copy constructor to construct inv. operator = is never called in this case.

Your second version works because

Inventory< std::string > inv;

calls the default constructor, and

inv = "a","b","c","a","c","d","e";

calls operator = on an already initialized object.

edited Oct 10 '12 at 12:36

answered Oct 10 '12 at 10:49

Luchian Grigore

253,575
64
457
625

Would it be possible to create a constructor that can handle a randomly sized list like that? (I'm toying with comma-operator to get a feel for what it can and cannot do, and where it would be useful.) – slashmais Oct 10 '12 at 11:08
Your explication is wrong. In this case, the comma isn't an operator at all, but a separator: `Inventory inv = "a", "b";` is the same as `Inventory inv = "a"; Inventory "b";` And the compiler doesn't like it if the name of a variable is a string constant. – James Kanze Oct 10 '12 at 11:11
@JamesKanze While that's true, I don't see how the explanation is wrong. – Luchian Grigore Oct 10 '12 at 11:12
@slashmais About the only thing overloading the comma operator can do is obfuscate. An overloaded operator should behave in a way corresponding to the built-in operator. Which means that you shouldn't overload operators which create sequence points. (Overloading `&&` or `||` is also a bad idea.) And in the case of comma, there's also the problem that sometimes it's an operator, and other times punctuation, and the overload is only applicable if it is an operator. – James Kanze Oct 10 '12 at 11:15
@LuchianGrigore You say that the compiler would first attempt to create a temporary `Inventory` from `"a","b","c","a","c","d","e"`. That's wrong: it will attempt to create a temporary from `"a"`, then to define a variable named `"b"` (which of course is illegal, since a string literal cannot be the name of a variable). – James Kanze Oct 10 '12 at 12:31
@JamesKanze aaa ok (I didn't compile the code), but yes, it's a declaration in the initialization case (I thought it would just use `"e"` to construct the temp). Will edit, thanks for pointing it out. – Luchian Grigore Oct 10 '12 at 12:36

score 2 · Answer 2 · answered Oct 10 '12 at 11:28

You're problem is that in one case, the comma is punctuation (and operator overloading doesn't apply), and in the other, it is an operator. The definition that doesn't work is basically the equivalent of:

Inventory<std::string> inv = "a";
Inventory<std::string> "b";
Inventory<std::string> "c";
//  ...

Because of this, overloading operator, is always bad design; when the comma is an operator, and when it isn't, is too subtle.

The usual way of doing something like this is to create a related class to collect the arguments, and pass it into the constructor:

Inventory<std::string> inv = Collector().add("a").add("b")...;

You could also overload an operator and use it, instead of the function add. But I don't see a likely operator (<<, inspired by ostream?) Or you could overload the constructor and the operator() of Collector, and write:

Inventory<std::string> inv = Collector("a")("b")...;

You would then use the same syntax for assignment. (You really don't want something that works for assignment, and not for copy construction.)

I'm inclined to agree that messing with the comma is a bad idea, for instance something like `int i; i = 1,2,3; //result: i == 1` and `i = (1,2,3); //result: i == 3` can become troublesome. Your `Collector`-idea seems like a handy one, I'll play that as well. — slashmais, Oct 10 '12 at 11:45

Variable initialization fails to compile with comma-operator

2 Answers2