This topic is 2649 days old which is more than the 365 day threshold we allow for new replies. Please post a new topic.

## Recommended Posts

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[][].

stejkenzie

##### Share on other sites
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 on other sites
So, what specifically don't you understand?

##### Share on other sites
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 on other sites
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 on other sites
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 == setpublic:	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)		//yet				soucet.prvky = new string[iter];							soucet.prvky[iter] = param.prvky;		}	}	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)				soucet.prvky = new string[iter];				soucet.prvky[iter] = this->prvky;		}	}	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;	}}void Mnozina::vypis(){				//print elements	for(int i = 0; i < pocetPrvku; i++){		cout << prvky << " ";	}}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 on other sites
"for(; iter < this->pocetPrvku; iter++){"

is this intentional?

##### Share on other sites
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)]
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 on other sites
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 on other sites
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;

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).

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 on other sites
Quote:
 Ain't there any way to use the built-in string + operator?

For string concatenation, yes. If you're building an array of strings it is quite useless unfortunately.

##### Share on other sites
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;	}	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?

##### Share on other sites
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.

Quote:
 Original Post by: rip-offRemember 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;

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 on other sites
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.