Question

Background

I have a container class which uses vector<std::string> internally. I have provided a method AddChar(std::string) to this wrapper class which does a push_back() to the internal vector. In my code, I have to add multiple items to the container some time. For that I have to use

container.AddChar("First");
container.AddChar("Second");

This makes the code larger. So to make it more easier, I plan to overload operator <<. So that I can write

container << "First" << "Second"

and two items will get added to underlying vector.

Here is the code I used for that

class ExtendedVector
{
private:
    vector<string> container;

public:
    friend ExtendedVector& operator<<(ExtendedVector& cont,const std::string str){
        cont.AddChar(str);
        return cont;
    }

    void AddChar(const std::string str)
    {
        container.push_back(str);
    }

    string ToString()
    {
        string output;
        vector<string>::iterator it = container.begin();
        while(it != container.end())
        {
            output += *it;
            ++it;
        }
        return output;
    }
};

It is working as expected.

Questions

  1. Is operator overload written correctly?
  2. Is it a good practice to overload operators in situations like this?
  3. Will there be any performance issues or any other issues with this code?

Any thoughts?

Edit

After hearing the excellent comments, I decided not to overload << as it doesn't make sense here. I removed the operator overload code and here is the final code.

class ExtendedVector
{
private:
    vector<string> container;

public:

    ExtendedVector& AddChar(const std::string str)
    {
        container.push_back(str);
        return *this;
    }

         .. other methods
}

This allows me to add

container.AddChar("First").AddChar("Second")

In C#, I can do this more easily by using the params keyword. Code will be like

void AddChar(params string[] str)
{
    foreach(string s in str)
       // add to the underlying collection
}

I know in C++, we can use ... to specify variable langth of parameters. But AFAIK, it is not type safe. So is it a recommended practice to do so? So that I can write

container.AddChar("First","Second")

Thanks for the replies.

Was it helpful?

Solution

Is operator overload written correctly?

It is, but one can do better. Like someone else mentioned, your function can be defined entirely out of existing, public functions. Why not make it use only those? Right now, it is a friend, which means it belongs to the implementation details. The same is true if you put operator<< as a member into your class. However, make your operator<< a non-member, non-friend function.

class ExtendedVector {
    ...
};

// note, now it is *entirely decoupled* from any private members!
ExtendedVector& operator<<(ExtendedVector& cont, const std::string& str){
    cont.AddChar(str);
    return cont;
}

If you change your class, you will not be sure that that your operator<< will still work. But if your operator<< entirely depends only on public functions, then you can be sure that it will work after changes were made to implementation details of your class only. Yay!

Is it a good practice to overload operators in situations like this?

As another guy said again, this is arguable. In many situations, operator overloading will look "neat" at first sight, but will look like hell next year, because you have no clue anymore what you had in mind when giving some symbols special love. In the case of operator<<, i think this is an OK use. Its use as an insertion operator for streams is well known. And i know of Qt and KDE applications that use it extensively in cases like

QStringList items; 
items << "item1" << "item2";

A similar case is boost.format which also reuses operator% for passing arguments for placeholders in its string:

format("hello %1%, i'm %2% y'old") % "benny" % 21

It's of course also arguable to use it there. But its use for printf format specifies are well known and so its use is OK there too, imho. But as always, style is also subjective so take it with a grain of salt :)

How can i accept variable length arguments in a typesafe way?

Well, there is the way of accepting a vector if you are looking for homogeneous arguments:

void AddChars(std::vector<std::string> const& v) {
    std::vector<std::string>::const_iterator cit =
        v.begin();
    for(;cit != v.begin(); ++cit) {
        AddChar(*cit);
    }
}

It's not really confortable to pass it though. You have to construct your vector manually and then pass... I see you already have the right feeling about the vararg style functions. One should not use them for this kind of code and only when interfacing with C code or debugging functions if at all. Another way to handle this case is to apply preprocessor programming. This is an advanced topic and is quite hacky. The idea is to automatically generate overloads up to some upper limit roughly like this:

#define GEN_OVERLOAD(X) \
void AddChars(GEN_ARGS(X, std::string arg)) { \
    /* now access arg0 ... arg(X-1) */ \
    /* AddChar(arg0); ... AddChar(arg(N-1)); */ \
    GEN_PRINT_ARG1(X, AddChar, arg) \
}

/* call macro with 0, 1, ..., 9 as argument
GEN_PRINT(10, GEN_OVERLOAD)

That is pseudo code. You can have a look at the boost preprocessor library here.

Next C++ version will offer far better possibilities. Initializer lists can be used:

void AddChars(initializer_list<std::string> ilist) {
    // range based for loop
    for(std::string const& s : ilist) {
        AddChar(s);
    }
}

...
AddChars({"hello", "you", "this is fun"});

It's also possible in next C++ to support arbitrary many (mixed-type) arguments using variadic templates. GCC4.4 will have support for them. GCC 4.3 already partially supports them.

OTHER TIPS

1) Yes, except since AddChar is public there's no reason it needs to be a friend.

2) This is arguable. << is sort of in the position of being the operator whose overloading for "weird" things is at least grudgingly accepted.

3) Nothing obvious. As always, profiling is your friend. You may want to consider passing the string parameters to AddChar and operator<< by const reference (const std::string&) to avoid unnecessary copying.

Is it a good practice to overload operators in situations like this?

I don't think so. It's confusing as hell for someone who doesn't know that you've overloaded the operator. Just stick to descriptive method names and forget about the extra characters you're typing, its just not worth it. Your maintainer (or you yourself in 6 months) will thank you.

I'd prefer not to overload it that way personally because vectors don't normally have an overloaded left shift operator - it's not really it's idiom ;-)

I'd probably return a reference from AddChar instead like so:

ExtendedVector& AddChar(const std::string& str) {
    container.push_back(str);
    return *this;
}

so you can then do

container.AddChar("First").AddChar("Second");

which isn't really much bigger than bitshift operators.

(also see Logan's comment about passing strings in by reference instead of by value).

The operator overloading in this case is not good practice as it makes the code less readable. The standard std::vector doesn't have it either for pushing elements, for good reasons.

If you're worried about the caller code being too long, you could consider this instead of the overloaded operator:

container.AddChar("First").AddChar("Second");

This will be possible if you have AddChar() return *this.

It's funny that you have this toString() function. In that case, an operator<< to output to a stream would be the standard thing to use instead! So if you want to use operators make the toString() function an operator<<.

The operator is not correctly overloaded here. There is no reason to make the operator a friend since it can be a member of the class. Friend is for functions which are not actual members of the class (such as when overloading << for ostream so the object can be output to cout or ofstreams).

What you actually want the operator to be:

ExtendedVector& operator<<(const std::string str){
    AddChar(str);
    return *this;
}

It is usually considered bad practice to overload operators in a way that has them do something than they do normally. << is normally bit shift, so overloading it this way can be confusing. Obviously STL overloads << for "stream insertion" and so along with that it might make sense to overload it for your use in a similar way. But that doesn't seem like what you're doing, so you probably want to avoid it.

There are no performance issues since operator overloading is the same as a regular function call, just the call is hidden because it is done automatically by the compiler.

This is going to make things rather confusing, I would use the same syntax as std::cin into a variable:

std::cin >> someint;

"First" >> container;

This way it is at least an insertion operator. To me when anything has a << overloaded operator I expect it to be outputting something. Just like std::cout.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top