Is this use of the "," operator considered bad form?

Question

I have made a list class as a means of replacing variadic functions in my program used for initializing objects that need to contain a changing list of elements. The list class has a usage syntax that I really like. However I haven't seen it used before, so I was wondering if I shouldn't use it just because of that fact? A basic implementation of the list class looks like this...

#include <list>
#include <iostream>
template<typename T>
struct list
{
    std::list<T> items;
    list(const list&ref):items(ref.items){}
    list(){}
    list(T var){items.push_back(var);}
    list& operator,(list add_){
        items.insert(items.end(),add_.items.begin(), add_.items.end());
        return *this;
    }
    list& operator=(list add_){
        items.clear();
        items.insert(items.end(),add_.items.begin(), add_.items.end());
        return *this;
    }
    list& operator+=(list add_){
        items.insert(items.end(),add_.items.begin(), add_.items.end());
        return *this;
    }
};

This allows me to have use this in code like so...

struct music{
//...
};
struct music_playlist{
    list<music> queue;
//...
};
int main (int argc, const char * argv[])
{
    music_playlist playlist;
    music song1;
    music song2;
    music song3;
    music song4;
    playlist.queue = song1,song2; // The queue now contains song1 and song2
    playlist.queue+= song1,song3,song4; //The queue now contains two song1s and song2-4
    playlist.queue = song2; //the queue now only contains song2
    return 0;
}

I really think that the syntax is much nicer than it would of been if I had just exposed a regular stl container, and even nicer (and typesafe) than variadic functions. However, since I have not seen this syntax used, I am curious about whether I should avoid it, because above all the code should be easily understood by other programmers?

EDIT:

In joint with this question, I have posted this question more targeted at solutions to the actual problem.

Answer 1

Why not overload the << operator as QList does? Then use it like:

playlist.queue << song1 << song2; // The queue now contains song1 and song2
playlist.queue << song1 << song3 << song4; //The queue now contains two song1s and song2-4

Answer 2

I agree that your syntax looks nice as you have written it.
My main difficulty with the code is that I would expect the following to be the same

playlist.queue = song1,song2;
playlist.queue = (song1,song2);  //more of c-style, as @Iuser notes.

whereas in fact they are completely different.

This is dangerous because its too easy to introduce usage bugs into the code. If someone likes to use parenthesis to add extra emphasis to groupings (not uncommon) then the comma could become a real pain. For example,

//lets combine differnt playlists
new_playlist.queue =    song1        //the first playlist
                      ,(song3,song4) //the second playlist //opps, I didn't add song 3!
                      , song5;        //the third 

or

new_playlist.queue = (old_playlist.queue, song6); //opps, I edited my old playlist too!

Incidently, have you come across boost.assign: http://www.boost.org/doc/libs/1_47_0/libs/assign/doc/index.html

Answer 3

Has the precedence changed recently?

playlist.queue = song1,song2;

This should parse as:

(playlist.queue = song1) , song2;

Your ',' and '+=' are the same! It would be a better semantic match if your comma operator were to create a temporary list, insert the left and right items and return the temporary. Then you could write it like this;

playlist.queue = (song1,song2);

with explicit parens. That would give C-programmers a fighting chance at being able to read the code.

Answer 4

A bit of a problem is that if the compiler cannot choose your overloaded operator comma, it can fall back on using the built-in operator.

In contrast, with Boost.Assign mixing up types produces a compilation error.

#include <boost/assign.hpp>

int main()
{
    int one = 1;
    const char* two = "2";
    list<int> li;
    li = one, two;

    using namespace boost::assign;
    std::list<int> li2;
    li2 += one, two;
}

Answer 5

This is probably something that belongs over on Programmers, but here's my two cents.

If you're talking about code that has a fairly narrow context, where users will use it in a couple of places and that's all, then overloading the , operator is probably OK. If you're building a domain-specific language that is used in a particular domain and nowhere else, it's probably fine.

The issue comes when you're overloading it for something that you expect the user to use with some frequency.

Overloading , means that the reader needs to completely reinterpret how they read your code. They can't just look at an expression and instantly know what it does. You're messing with some of the most basic assumptions that C++ programmers make when it comes to scanning code.

Do that at your own peril.