Adding two arrays to temp array, what's wrong here?

Pages: 12
skatingrocker17 (128)
I'm trying to a add two arrays of chars together into an array called Temp. The second array leaves off where the first ended so all the contents are together in one array.

This is a function of a class so it might be syntactically correct but I'm getting some errors and I'm not sure why, here's what I've got...
1
2
3
4
5
6
7
8
9
10
11
12
13
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
	Temp[i] = Bag one[i];


for (i = i; i < num; i++)
	Temp[i] = Bag two[i];

return Temp;
}



Here's my errors:
~/cs2020c$ g++ prog5.cpp bag.cpp 
bag.cpp: In member function 'Bag Bag::operator+(Bag&)':
bag.cpp:86: error: no match for 'operator[]' in 'Temp[i]'
bag.cpp:86: error: expected primary-expression before 'one'
bag.cpp:86: error: expected `;' before 'one'
bag.cpp:90: error: no match for 'operator[]' in 'Temp[i]'
bag.cpp:90: error: expected primary-expression before 'two'
bag.cpp:90: error: expected `;' before 'two'



And finally, here's what it's supposed to do:
Function: operator+ // add two bags
Parameters: Bag
Description: creates new bag containing all the characters in both bags
Returns: resulting Bag



hamsterman (4538)
lines 6, 10: You probably don't have operator [] overloaded for Bag. Also, Bag one[i]; makes no sense. This is a declaration of an array of i Bags, which, of course, is not valid because i is not a constant and you have placed it on the right side of =. I have no clue what you wanted to do here. Post the declaration of Bag class.

Also, note that i=i makes no sense (you can leave that place empty, if you want) and that both loops have the same condition, so when the first ends, the second won't start (because i will be equal to num).
skatingrocker17 (128)
Thanks for your help hamsterman.

You're right. Okay I have 2 bags initialized already; Bag one and Bag two. The goal with this function is to (hopefully) add the contents of both bags into a temporary bag and return it. In the Bag class, "num" is the number of items in the array and the array is named "contents".

So the question is, how to I had both arrays together into a temporary bag?


I've made some changes but still a bit confused..
1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
	Temp[i] = Robj.contents[i];


//for (i = i; i < num; i++)
//	Temp[i] = Bag two contents[i];

return Temp;
}

~/cs2020c$ g++ prog5.cpp bag.cpp
bag.cpp: In member function 'Bag Bag::operator+(Bag&)':
bag.cpp:86: error: no match for 'operator[]' in 'Temp[i]'

Last edited on
hamsterman (4538)
As I said.
You don't have the operator [] overloaded for Bag so you cant do some_bag[i]. Instead it should be some_bag.member_array[i]. I guess that "member_array" is "contents" but I can't be sure as I don't see your Bag class declaration.
skatingrocker17 (128)
Well here's the .h file.
I'm still confused on the overloaded instructors, everything I have is from the examples I've been referencing. Any help is greatly appreciated. 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
 
#include <iostream>
#include <string>
 
using namespace std;
 
class Bag {
	friend istream &operator>>( istream &input, Bag &Robj );
	friend ostream &operator<<( ostream &output, Bag &Robj );
 
public:
    Bag ();
    Bag(string);
    Bag operator+( char addchar);  //add a char to the array
    Bag operator+(Bag &Robj);//add two arrays together
private:
    char contents[1024];
    int num;
    void compact();
    bool hasA(char X);	//determines if the char is in the array (somewhere)
};



Last edited on
hamsterman (4538)
Ok. What you want to do is take this->contents, Robj.contents and put them both in Temp.contents .
It is good that you assign *this to Temp, as it copies all contents of *this. Now Temp.contents has whatever this->contents had, and Temp.num == this->num. The question is, where should you write the contents of Robj? In your previous attempt you did simple copying and thus rewrote whatever came from this->contents.

Example
this->contents = "hello ", this->num = 6
Robj.contents = "world", Robj.num = 5
after assigning *this to temp,
Temp.contents = "hello ", Temp.num = 6

now you need to write Robj somewhere
starting from Robj.contents[0], which is 'w'

if you write it into Temp->contents[0], you'll get "wello ".
that's not right. the first free position is 6. when that one is taken, 'o' will go to 7, etc.
now, where did 6 come from? it is Temp.num

thus we can see that Robj.contents[i] is written into Temp.contents[Temp.num+i] where i goes from 0 to Robj.num
don't forget to increase Temp.num after you've done copying.
skatingrocker17 (128)
That makes senses and clears up a few things for me.

In the client program, this is what the operation looks like..
1
2
3
4
 
// *** testing + to add two bags together
    one = one + two;
    cout << "******* *** testing + to add two bags \n";
    cout << one ;



I'm wondering if I'm even explaining this right or if the function is going to do the right thing.

Currently I have this which I think is putting all of Bag one in temp but whtat about Bag two? Do I need another for loop?
1
2
3
4
5
6
7
8
9
10
11
12
13
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
	Temp.contents[i] = Robj.contents[i];


//for (i = i; i < num; i++)
//	Temp[i] = Bag two contents[i];

return Temp;
}





I just discovered there's something wrong with my constructor (I think), this program has two constructors and the more difficult was was done for mee but I'm not quite sure how to even make one for this situation.
Default Constructor
Description: the resulting Bag should be empty.

Note: a second constructor is provided for you. It takes a string as a parameter and copies the characters in the string into the constructed Bag.


Last edited on
hamsterman (4538)
Currently I have this which I think is putting all of Bag one in temp but whtat about Bag two? Do I need another for loop?
Robj.contents[i] is written into Temp.contents[Temp.num+i] where i goes from 0 to Robj.num


Your default constructor will have to set num to 0. That is all.
skatingrocker17 (128)
Is this what you mean?
1
2
3
4
5
6
7
8
9
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
	Temp.contents[Temp.num + i] = Robj.contents[i];

return Temp;
}



Surprisingly, I actually had the constructor right, setting num to 0.

The instructor created some of the client and class files and as we implement more functions into the class we just uncomment lines in the client program to use them.

But the fun part is, trying to guess what this means. I've seen this before and it's always something different.
prog5.cpp: In function 'int main(int, const char**)':
prog5.cpp:25: warning: the address of 'Bag one()', will always evaluate as 'true'
Undefined                       first referenced
 symbol                             in file
operator<<(std::basic_ostream<char, std::char_traits<char> >&, Bag&)/var/tmp//ccHxJZiF.o
ld: fatal: Symbol referencing errors. No output written to a.out
collect2: ld returned 1 exit status

hamsterman (4538)
Is this what you mean?
Almost. Say, on line 5, what is num, and why is it there. Also try thinking about what would happen if you tried joining "" with "some word".

But the fun part is, trying to guess what this means.
Error messages can indeed be hard to understand. This sounds like you're treating &one as a boolean somewhere, but I can't tell without seeing your main().
skatingrocker17 (128)
Okay, it seems like I'm about halfway there.

Here's the code of what it's supposed to do:
1
2
3
4
 
// *** testing + to add two bags together
    one = one + two;
    cout << "******* *** testing + to add two bags \n";
    cout << one ;


Here's my class function:
1
2
3
4
5
6
7
8
9
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
	Temp.contents[Temp.num + i] = Robj.contents[i];

return Temp;
}


Here's my output when I run it:
Welcome to the bag operator driver
******* cout test: 

a b b c c c d d d d 1 
******* single char addition test: 
A B C 
******* *** testing + to add two bags 
A B C 
Not found
Not found
Not found
******* ***


And here's the CORRECT output:
Welcome to the bag operator driver
******* cout test: 

a b b c c c d d d d 1 
******* single char addition test: 
A B C 
******* *** testing + to add two bags 
A B C a b b c c c d d d d 1


Last edited on
closed account (3TXyhbRD)
1
2
3
4
5
6
7
8
9
 
Bag Bag::operator+(Bag &Robj)
{
    Bag Temp = *this; 
    int i = 0;
    for (i = 0; i < num; i++)
        Temp.contents[Temp.num + i] = Robj.contents[i];

    return Temp;
}

Which num? Where does the string end? You don't reassign NULL to sign the end of the string. And in the above code, num is actually this->num.

I don't see any further errors with this code, however I would like to see your constructors code [note the various string types (char*, w_chart*, string, System::String, System::String^ are the ones I came across so far) that you will need to treat in your constructor code, it's a really good exercise].
Last edited on
skatingrocker17 (128)
Here's all the code:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
 
//  bag.h
//
//
  
#include <iostream>
#include <string>
 
using namespace std;
 
class Bag {
	friend istream &operator>>( istream &input, Bag &Robj );
	friend ostream &operator<<( ostream &output, Bag &Robj );
 
public:
    Bag ();
    Bag(string);
    Bag operator+( char Y);  //add a char to the array
    Bag operator+(Bag &Robj);//add two arrays together
    Bag operator-( char Z);
    int getCount();
private:
    char contents[1024];
    int num;
    void compact();
    bool hasA(char X);	//determines if the char is in the array (somewhere)
};
 
 

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
 
//
//  bag.cpp
//
//
 
#include "bag.h"
 

Bag::Bag( )
{
    num = 0;  
}


/*  Constructor, secondary.
    fill a bag up with the characters from a string.
*/ 
Bag::Bag(string source) {
    for (int i=0; i < (int)source.length(); i++)
        contents[i]=source[i];
    num = (int)source.length();
}

/*
  isAlNum  determines if a character is alphanumeric.
  called from Bag:compact
*/
bool isAlNum( char X) { 
    if ( X >= 'A' && X <= 'Z') return true;
    if ( X >= 'a' && X <= 'z') return true;
    if ( X >= '0' && X <= '9') return true;
    
    return false;
}
 
/*  compact the Bag's internal array by removing all non alphnumeric chars
    up to position num 
*/
void Bag::compact() {
   int last = num-1;
    for (int i=num-1; i>=0; i--) {
        if ( !isAlNum( contents[i] ) )  {
             contents[i] = contents[last];
             last--;
        }
    num = last+1;
    
  }
}

bool Bag::hasA (char X)
{
	int i = 0;
	
	for (i = 0; i < num; i++)
		{	
			if (contents[i] == X)
				return true;
		}


	return false;
}


Bag Bag::operator+( char Y)
{  
	Bag Temp = *this; 
	
	
	Temp.contents[Temp.num++] = Y;

    return Temp;      //Return copy of local variable.
                           
}




Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
	Temp.contents[Temp.num + i] = Robj.contents[i];

return Temp;
}



Bag Bag::operator-( char Z)
{  
	Bag Temp = *this; 
	int i = 0;	
		
	/* for (i = 0; i < num; i++)
	{
		if (Temp.contents[i] == Z)
		{
			contents[i]=contents[i+1];
		}
		else
			cout << "Not found" << endl;
	} 
*/
	for (int j = 0; j < num; i++)
		{
			if (Temp.contents[i] == Z){
				j = i;
	for (i = (Z-1); i < num; ++i)
    		Temp.contents[i] = Temp.contents[i + 1]; }}

	

    return Temp;      //Return copy of local variable.
                           
}

int getCount ()
{	
	int i = 0;
	int count = 0;
	
	for (i = 0; i < 1024; i++) //Still need to figure out how many items are in bag....
		count ++;

	return count;	
}
	
	
	
	
ostream &operator<<(ostream &output,  Bag &Robj )
{
   int i = 0;
   for (i=0; i < Robj.num; i++)
		output << Robj.contents[i] << " ";
   output << endl;

   	return output;   // enables cascading
}





1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
 
//
// 
// 
// 
// 
//
// Note all includes are 'included' from bag.h
//
// lines that begin with // ***    are "real" comments
// lines with just //  are for you to uncomment as you implement the functions
 
#include "bag.h"
 
int main (int argc, const char * argv[])
{
    cout << "Welcome to the bag operator driver" << endl;

// *** Create two bags.
    Bag two("abbcccdddd1");

    Bag one;

// *** testing << with cout.
  cout << "******* cout test: \n";
  cout << one << two;
    
// *** testing + to add single chars to a bag
   one = one + 'A';
   one = one + 'B';
   one = one + 'C';
   cout << "******* single char addition test: \n";
   cout << one ;
    
// *** testing + to add two bags together
    one = one + two;
    cout << "******* *** testing + to add two bags \n";
    cout << one ;
    
// *** testing - to remove a char instance from a bag
    two = one - 'd';
    cout << "******* *** testing -  to remove a 'd' from the bag \n";
    cout << two ;

// *** testing - to remove the first element 
//    two = one - 'A';
    cout << "******* *** testing -  to remove the first element " << endl;
//    cout << two ;
    
// *** testing - to remove a char NOT in the bag
//    two = two - 'Q';
    cout << "******* *** testing -  to remove a Q (not in bag) \n";
//    cout << two ;
    
// *** testing  hasA  using a char NOT in the Bag
    cout << "****** *** testing hasA.  Is there a Z in this Bag?\n";
//    cout << "     " << one;
//    if ( one.hasA('Z' ) ) 
//        cout << " Yes. the bag has a Z\n";
//    else
//        cout << " No. the bag does not have a Z\n";
    
// *** testing  hasA  using a char n the Bag
//    one = one + 'Z';
//    cout << "****** *** testing hasA.  Is there a Z in this Bag?\n";
//    cout << "     " << one;

//    if ( one.hasA('Z' ) ) 
//        cout << " Yes. The bag has a Z\n";
//    else
//        cout << " No. THe bag does not have a Z\n";
   
//  *** testing  getCount 
    cout << "****** *** testing getCount on Bag\n";
//    cout << "Bag contents: " << one;
//    cout << "The count is: " << one.getCount() << endl;

    return 0;
}




closed account (3TXyhbRD)
Mighty thanks. You need to work more on char strings. As far as my eye could read, you don't mark the end of the string with either 0, NULL or '\0'. Also you got arrays when you could use simple characters, but let's not hit optimizing yet.

Here's a simple example that will transform (even tho there is a function, I know) an int into an char*. Also with the exclusion of negative numbers.
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
 
#include <iostream>

using namespace std;

int cif(int n)
{
   //Declarations
   int s = 0;

   //Loop for getting the number of letters <n> contains
   do{
      n = n/10;
      s++;
   }while(n);

   //Return the number of letters
   return s;
}

void main()
{
    //Declarations
    int x, len;
    char a[10];

    //Read <x>
    cout<<"x=";cin>>x;

    /*Small theory
    So C++ arrays start from 0 index
    that means that if <x> is a 3 letter number
    we will have our array till index 2
    however we need NULL to sign the end of array
    NULL is right after the last letter meaning it's exactly
    on the number of letters <x> has index
    Small theory over, sorry for bad and clouded English*/

    //Store the number of letter <x> has
    len = cif(x);
    //Place NULL to sign the end of the array
    a[ len ] = 0;
    //Drop <len> by 1 so you will be on the right index
    len--;

    //Transform the number into an array
    do{
        //<array> [ <index> ] = <last letter> + <ASCII code of 0 (zero, don't confuse with NULL)>;
        //Addition of '0' ASCII code to get the letter ASCII code that is stored in the array
        a[len] = (x%10) + '0';

        //Get rid of the last letter
        x = x/10;

        //Drop the index by 1
        len--;
    }while(x);

    //Write the array
    cout<<a;
}
Some comments on the source code are not necessary, you can just ignore them if you want to.

I haven't compiled it but it should work. Take a look at how the array is being made because in your constructor code that has string, you do something like this, only that you won't need to add '0' ASCII code, you just copy them over.
skatingrocker17 (128)
How do I add the two arrays though?

I could integrate some of what I've learned from you but I'm not doing any extra work. I'm not even familiar with a lot of your code because we haven't learned it yet.
Last edited on
Veltas (450)
1
2
3
4
5
6
7
8
9
 
Bag Bag::operator+(Bag &Robj)
{
	Bag Temp = *this;
	// It starts off with the other bag's contents,
	// so starting from the end of the list that's already there...
	for (int i = Temp.num; i < Temp.num + Robj.num; ++i) Temp[i] = Robj.contents[i - Temp.num];
	Temp.num += Robj.num; // Compound assignment, x += y is the same as x = x + y
	return Temp;
}


Would that do? It adds the lists together and ammends num to represent the new size... I think.

EDIT: And please change num to an unsigned int. You're wasting that precious sign bit on something that should only be positive (I think).

EDIT EDIT: Hold on, provided your strings are null-terminated then each string should end with a null character (char nullChar = 0), so you have no need for storing the sizes, you might as well do this instead:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
Bag Bag::operator+(Bag &Robj)
{
	Bag Temp = *this;
	// Find the end of the last list.
	int num;
	for(num = 0; Temp.contents[num]; ++num);
	// Ends when null character reached as a null works as a false (as it is equivilant to integer 0).
	// Now copy next half over.
	int i;
	for (i = 0; Robj.contents[i]; ++i) Temp[num + i] = Robj.contents[i];
	// And remember to add a null to the end.
	Temp[num + i] = 0;
	return Temp;
}


I hope that works...
Last edited on
closed account (3TXyhbRD)
Veltas code is almost good, it misses NULL character, end of array everyone.

The bellow code should do what you want, however you have constructor code that isn't correct. You need to get a better hold on char strings to be able to make a class that uses char strings.
1
2
3
4
5
6
7
8
9
10
11
12
13
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
/*for (i = 0; i < num; i++)
	Temp.contents[Temp.num + i] = Robj.contents[i];*/

for (i = 0; i <= Robj.num; i++)
	Temp.contents[Temp.num + i] = Robj.contents[i];
Temp.num = Temp.num + Robj.num;

return Temp;
}
You're learning really jumpy and it's gonna cost you later. PM me what you have gone through so far.
skatingrocker17 (128)
Thanks Veltas. I copied that over and got the following two errors.
~/cs2020c$ g++ prog5.cpp bag.cpp 
bag.cpp: In member function 'Bag Bag::operator+(Bag&)':
bag.cpp:102: error: no match for 'operator[]' in 'Temp[(((Bag*)this)->Bag::num + i)]'
bag.cpp:103: error: no match for 'operator[]' in 'Temp[(((Bag*)this)->Bag::num + i)]'


I changed it a little and got the same thing (I think it's the still the same as your code, just cosmetically a little different).
1
2
3
4
5
6
7
8
9
10
11
12
13
 
Bag Bag::operator+(Bag &Robj)
{
	Bag Temp = *this;
	// Find the end of the last list.
	//int num;
	for(num = 0; Temp.contents[Temp.num]; ++num);
	// Ends when null character reached as a null works as a false (as it is equivilant to integer 0).
	// Now copy next half over.
	int i;
	for (i = 0; Robj.contents[i]; ++i) Temp[Robj.num + i] = Robj.contents[i];
		Temp[num + i] = 0;	// And remember to add a null to the end.
	return Temp;
}

Veltas (450)
@LilJoe Sorry, I realised that they were strings and ammended my post to include a version that relies on standard null-terminated strings instead of storing the lengths.

@skatingrocker17 Excuse me, I'm still quite new to C++ (as you may be able to tell).

I accidentally forgot to address the array correctly and forgot something else was already called num (great minds think alike).

Here is the correct code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
 
Bag Bag::operator+(Bag &Robj)
{
	Bag Temp = *this;
	// Find the end of the last list.
	int j;
	for(j = 0; Temp.contents[j]; ++j);
	// Ends when null character reached as a null works as a false (as it is equivalent to integer 0).
	// Now copy next half over.
	int i;
	for (i = 0; Robj.contents[i]; ++i) Temp.contents[j + i] = Robj.contents[i];
	// And remember to add a null to the end.
	Temp.contents[j + i] = 0;
	return Temp;
}


Note that I have not used num anywhere, this is because num is not necessary if you use null-terminated strings. http://www.cplusplus.com/doc/tutorial/ntcs/
Last edited on
skatingrocker17 (128)
You're code is right, but the way my program is set up, it didn't work.

I sent my code to my instructor a little bit ago for some advice and he said:
You are almost there, the question is who "num" is that in the for loop,
which object does it belong to.


For reference, here's what I sent him:
1
2
3
4
5
6
7
8
9
 
Bag Bag::operator+(Bag &Robj)
{
Bag Temp = *this; 
int i = 0;
for (i = 0; i < num; i++)
        Temp.contents[Temp.num + i] = Robj.contents[i];

return Temp;
}
Pages: 12