Sign in to follow this  
stejkenzie

Overloading + (plus) operator

Recommended Posts

stejkenzie    109
Hi everyone,

I'm doing some college work for a friend and I've come to problem I'm short to solve.
The trouble point is:
I'm supposed to overload + (plus) operator so it unites two sets. Both sets are string arrays.
That means if a.elements string array contains "1" and "x" and b.elements string array contains "2", "y" and "x", c = a + b should make c.elements "1", "x", "2", "y".
Both string arrays are created in heap.
I can copy the whole code if needed.
PS: I can't use containers.
PPS: I'm using string type, not char[][].

Thanks for any advices.
stejkenzie

Share this post


Link to post
Share on other sites
rip-off    10976
We cannot give you an answer for homework questions. If you have an implementation that is nearly there, and you have a specific problem with it, then we can give you hints as to how to proceed.

Your "friend" would be better served by doing their own homework.

Share this post


Link to post
Share on other sites
stejkenzie    109
2rip-off: That's what would I say to him if I wasn't newcomer to C++ on my own. I need as many experience I can get.

2nyxor: I need little chunk of code that would solve this problem. An explanatory one so I would understand it in first place, not just copy it.

I've never overloaded operator before and the examples I found are either with integers or with different operators.

Share this post


Link to post
Share on other sites
rip-off    10976
You cannot overload an operator on a built-in type, such as arrays of strings.

You could create a set<> class, then overload the operator on that type:

class set {
// ...
};

set operator+(const set &a, const set &b)
{
// union implementation
}


Note that operator overloading is not a great idea on types where there is no convention for what it means. I would use an explicit "set_union" function were I implementing this (I would use "union" but it is a keyword).

When you say you cannot use containers, does that mean you cannot use the existing set<> class in the Standard C++ Library?

Share this post


Link to post
Share on other sites
stejkenzie    109
I commented the code so you would (hopefully) understand.
It's a bit longer, but I think most of it makes sense.
I've got many ideas of how to overload the operator, but none of them seems to work and the program always crashes on the same line which is c = a + b;
Please, analyze this and gimme some hint or code for the operator.


#include <iostream>
#include <sstream>
using namespace std;

class Mnozina{ //mnozina == set
public:
Mnozina();
Mnozina(int countPrvku); //takes parameter number of elemets
void vypis(); //print elements
int getVelikost(); //get size - not implemented yet
Mnozina operator+ (Mnozina param);
private:
int pocetPrvku; //number of elements
string* prvky; //elements stored in the array of strings on the heap
};
Mnozina::Mnozina(){ //i'm not sure if this constructor shouldn't be doing something
}
Mnozina Mnozina::operator+ (Mnozina param){ //this is the main problem
Mnozina soucet; //temp set
int iter; //number of elements of the new string array
for(iter = 0; iter < (param.pocetPrvku); iter++){ //copies all elements of b set to tep set
for(int i = 0; i <= iter; i++){ //if the element is not there
if(soucet.prvky[iter] != param.prvky[i]) //yet
soucet.prvky = new string[iter];
soucet.prvky[iter] = param.prvky[i];
}
}
for(; iter < this->pocetPrvku; iter++){ //the same code as above, but adds it to the end (like vector pushack())
for(int i = 0; i <= iter; i++){
if(soucet.prvky[iter] != this->prvky[i])
soucet.prvky = new string[iter];
soucet.prvky[iter] = this->prvky[i];
}
}
return soucet;
}

Mnozina::Mnozina(int countPrvku){ //prompt for elements
pocetPrvku = countPrvku;
prvky = new string[pocetPrvku];
for(int i = 0; i < pocetPrvku; i++){
cout << "prvek " << i + 1 << "/" << pocetPrvku << " ";
cin >> prvky[i];
}
}
void Mnozina::vypis(){ //print elements
for(int i = 0; i < pocetPrvku; i++){
cout << prvky[i] << " ";
}
}

int main(int argc, char **argv){ // main() is assigned, there can be no changes in here
if(argc != 3) {
cerr << "Program prijima 2 short parametry" << endl; //"program take 2 short parameters"
return 0;
}
cout << "Zadejte prvky pro mnoziny a a b:"<< endl; //"enter elements for sets a and b"
Mnozina a(atoi(argv[1])); Mnozina b(atoi(argv[2]));
Mnozina c,d,e,f;
cout << "Prvky a: "; a.vypis(); cout << endl;
cout << "Prvky b: "; b.vypis(); cout << endl;
cout << "Sjednoceni c=a+b: ";
c=a+b; c.vypis(); // this is the point where the program fails

}



Thanks, stejkenzie

Share this post


Link to post
Share on other sites
stonemetal    288
The thing to remember about operator overloading is that is exactly writing any other function in C++. If you know the behavior you want and could write the function if it were just called add, then that is exactly what you would write for operator +.

if(soucet.prvky[iter] != param.prvky[i])]
are we sure this line is valid? you just instantiated soucet a few lines back and your constructor doesn't set prvky to point to anything so that pointer isn't valid.

Share this post


Link to post
Share on other sites
stejkenzie    109
2Kalten: it was, until you pointed it out. Now I can see it's not actually working, cuz of the condition.

2stonemetal: nice, I didn't notice that one. I'm creating first element after I check if it doesn't contain some value. Dammit!

Still, I kinda feel like this is the worst piece of code you could ever write for operation like this. Ain't there any way to use the built-in string + operator?

[Edited by - stejkenzie on November 15, 2010 3:03:11 AM]

Share this post


Link to post
Share on other sites
rip-off    10976
Your if statements lack braces, the indentation does not match what the compiler sees.

Here is another problem:

soucet.prvky = new string[iter];
soucet.prvky[iter] = this->prvky[i];

You aren't allocating enough objects to index prvky like this. Remember that if you have an array of size N you can only index from 0 to N - 1.

One thing you might consider is to separate the memory logic from the set logic. You could use std::vector<> as a placeholder until you have the set working, then replace this with a custom dynamic array class if that is part of your requirements.

Your algorithm is complex because your set class does not enforce uniqueness. If you write a memeber function too add new elements that will discard duplicate elements, this could be used in the constructor and in operator+ (remember: such an invariant must be enforced *everywhere* elements are added).

Your algorithm could then be:

Set operator+(const Set &a, const Set &b)
{
Set result = a;
for each(Element e in b)
{
result.add(e);
}
return result;
}

Note that this is a free function, it does not need to access private data.

Share this post


Link to post
Share on other sites
stejkenzie    109
Thanks for ideas, but unfortunately they don't work. The + operator can take only one argument, so (const Mnozina& a, const Mnozina& b) ain't possible. But I've realized, that it was pretty stupid to do iterations like I had before, so I changed the code to:

Mnozina Mnozina::operator+ (const Mnozina& right){	//this is the main problem
Mnozina temp;
temp.prvky = right.prvky;
temp.pocetPrvku = right.pocetPrvku;
for(int i = 0; (i < this->pocetPrvku); i++){
temp.prvky = new string[temp.pocetPrvku + i];
temp.prvky[i + temp.pocetPrvku] = this->prvky[i];
}
return temp;
}


Seems quite a logical to me, but there's one error and I don't know why does it happen - last line in the for loop give is bad pointer for [i + temp.pocetPrvku]. I don't know why.
I assign right.pocetPrvku (lets say 3), to temp, then create a new string on the heap (it's not what the function is supposed to do, but right now I'm trying to at least merge those two arrays together) and then assign a value to that string. That means (if we work with 3) I'm creating string on the 3rd position in the array and then I initialize it with what's in this->prvky[0]. Where's my mistake? How come I can create string on the [temp.pocetPrvku + i] position but I can't initialize it using the same formula?

Thanks for answers.

Share this post


Link to post
Share on other sites
rip-off    10976
Quote:

Thanks for ideas, but unfortunately they don't work. The + operator can take only one argument, so (const Mnozina& a, const Mnozina& b) ain't possible.

It will work, if you implement operator+ as a free function (i.e. a non-member function). In this case it takes two parameters. Note how in my example operator+ was in the global namespace, not a member of the Set class (its signature would have been Set Set::operator+(const Set &other)).

You should only need to make a single allocation, once before the loop. Allocating every iteration means you leak memory. Your code can cause a shallow copy sometimes, as well as a deep copy, this is a source of confusing bugs.

Set union is usually handled without duplicates. Your code does not appear to handle this case.

However, for your specific error:
Quote:
Original Post by: rip-off
Remember that if you have an array of size N you can only index from 0 to N - 1.

So look at the code:

temp.prvky = new string[temp.pocetPrvku + i];
temp.prvky[i + temp.pocetPrvku] = this->prvky[i];

The first line allocates an array of size temp.pocetPrvku + i elements. The next line tries to assign an element outside the bounds of that array.

Remember, new[] allocates a big array. It doesn't append to the end of an existing array, if that was what you were hoping.

Share this post


Link to post
Share on other sites
Aspirer    544
Since I'm typically against handing someone a detailed answer so they can just copy/paste it and turn it in to their professor, I'm going to say this:

www.cprogramming.com offers a very good, detailed tutorial on Overloading Operators. I would suggest you start there, as by the time you finish with the tutorial, you should have a good enough understanding that you can easily apply this to your class.


Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this