0

I have overloaded + operator for user-defined StringSet class where it unites two StringSets.

StringSet& StringSet::operator+(StringSet& set2) {
    StringSet* temp = new StringSet;
    for (auto i = 0; i < this->getSize(); i++) {
        temp->addSet(this->data[i]);             
    }
    for (auto j = 0; j < set2.getSize(); j++) {
        temp->addSet(set2.data[j]);              
    }
    return *temp;
}

If I don't allocate temp dynamically am unable to use it as

cout << "union is: " << s1+s2 << endl;    // s1 and s2 are StringSet objs created using constructor

The current code works fine for this but, I wonder if I can do this without alloacating temp dynamically or allocating then deleting it in some way.

2 Answers2

5

Dynamic allocation is completely misguided here. You need to return by value:

StringSet StringSet::operator+(StringSet const& set2) const {
    StringSet temp;
    for (auto i = 0; i < this->getSize(); i++) {
        temp.addSet(this->data[i]);             
    }
    for (auto j = 0; j < set2.getSize(); j++) {
        temp.addSet(set2.data[j]);              
    }
    return temp;
}

(Also note the const& parameter and const function declaration!)

The same is true not just for concatenation but whenever you create and return a new object (except of course for polymorphic types, where you need to use indirection via a pointer or similar).

Furthermore, as noted in a comment by user idclev, it’s customary to implement operator+ in terms of operator+=:

StringSet StringSet::operator+(StringSet const& other) const {
    auto result = *this; // This invokes the copy constructor!
    return result += other;
}

StringSet& StringSet::operator+=(StringSet const& other) {
    for (auto i = 0; i < other.getSize(); ++i) {
        addSet(other.data[i]);
    }
    return *this;
}
Konrad Rudolph
  • 530,221
  • 131
  • 937
  • 1,214
  • but returning by value don't support concatenation – letsShareKnowledge Oct 16 '20 at 09:34
  • @letsShareKnowlede What do you mean by that? – Konrad Rudolph Oct 16 '20 at 09:35
  • I mean if the function is returning by value then this line "cout << "union is: " << s1+s2 << endl;" don't woks. I was wondering if there is a way to do so – letsShareKnowledge Oct 16 '20 at 09:38
  • 2
    @letsShareKnowlede No, of course that line will work (that’s the whole point), what makes you think it wouldn’t? If the line doesn’t work in your code then that’s caused by the fact that you failed to pass the object as `const&` in your `operator<<` overload. Always be mindful of your `const`s! – Konrad Rudolph Oct 16 '20 at 09:38
1

Providing additional overloads for moves could result in performance improvements (refer to this link)

In this case, you can implement the overload + operator with rvalue reference as follow:

StringSet&& StringSet::operator+(StringSet&& set2) {
    StringSet temp;
    for (auto i = 0; i < this->getSize(); i++) {
        temp.addSet(this->data[i]);             
    }
    for (auto j = 0; j < set2.getSize(); j++) {
        temp.addSet(set2.data[j]);              
    }
    return std::move(temp);
}