Trouble with a function

[kenji]

Hi,

Im having some trouble with the function and I hope some one can help.

The function is supposed to go through an class array and loop for an empty spot. If it finds one its copies data to the data members.

Otherwise it creates a temporary array and copies the values and dynamically increases the array size by 10 and then it copies back the data from the temporary array.

My problem is that once I create the dynamic array I am stuck in the for loop and the arraysize keeps increaseing.

const char* Bank::operator+=(const char data[]){
	int i = 0;
	const int len = arraySize;
	Account *temp;
	temp = NULL;
	temp = new Account[len];
	int bal;
	char anum[15 + 1];
	char cust[315 + 1];
	
	sscanf(data,"%15[^,],%d,%315[^;];", anum, &bal, cust);

	int value = strlen(cust);
	cust[value] = ';';
	cust[value + 1] = '\0';
	cout << "1: " <<arraySize << endl;

	while(savings[i].accountNumber != NULL){
		i++;
	}

	if(savings[i].accountNumber == NULL){
		strcpy(savings[i].accountNumber, anum);
		savings[i].balance = bal;
		strcpy(savings[i].customer, cust);
		size++;
		cout << "2: " << arraySize << endl;
	}

	else{
		while(i < arraySize){
			strcpy(temp[i].accountNumber, savings[i].accountNumber);
			temp[i].balance = savings[i].balance;
			strcpy(temp[i].customer, savings[i].customer);
			i++;
		}
			
		delete [] savings;

		savings = new Account[arraySize + 10];

		i = 0;
		for(i; i < len; i++){
			strcpy(savings[i].accountNumber, temp[i].accountNumber);
			savings[i].balance = temp[i].balance;
			strcpy(savings[i].customer, temp[i].customer);
		}
		delete [] temp;
		}
		strcpy(savings[i].accountNumber, anum);
		savings[i].balance = bal;
		strcpy(savings[i].customer, cust);

		size++;
		arraySize = arraySize + 10;
		return data;
}

[StuXYZ]

Ok without a lot more of this program I am guessing. BUT it is obvious that many things need to be fixed.

(a) The start? const int len=arraySize; you don't change arraySize until the last line ? so why len?

(b) The first while loop [CHECK for i>=number of savings]:
Try:

while(i<numberofSavings &&  savings[i] &&
         !savings[i].accountNumber)
{ 
      i++;
}

The next if (savings[i].accountNubmer==NULL) well it can't possibly be true in your code.

Then is gets REALLY scary:
You delete ALL savings and then create saving +10 .... (this isn't a copy) and then you assume that everything is ok
Then we see that you allocated temp REGARDLESS of what you thought you were doing, and you are hoping that constructor populated everything perfectly???

Finally, you input data and return data WHY?

Summary:

This is such a mess. You should post the whole project. BUT before you do you should format it differently. It should have each { and } on a new line. It should be space indented so you can see your loops.

You should look at breaking things into smaller functions. E.g. Adding an extra 10 accounts. Do this with a function, then the coping etc. can be done away from the other logic and then you might be able to follow your own code.

It is simple you HAVE to simplify your code so you can easily read it thought and know how the variables and class variables in a method change.

[kenji]

I made a mistake with the above code, I had changed a lot of the code but copied the wrong file.

Basically what I have to do is search an dynamically allocated array to find an empty space and if I find one I copy data to it, otherwise I need to create a temporary array and copy data that and increase the array length of the dynamically allocated array called savings.

const char* Bank::operator+=(const char data[]){
	int i = 0;
	const int len = arraySize;
	Account *temp;
	temp = NULL;
	temp = new Account[len];
	int bal;
	char anum[15 + 1];
	char cust[315 + 1];
	
	sscanf(data,"%15[^,],%d,%315[^;];", anum, &bal, cust);

	int value = strlen(cust);
	cust[value] = ';';
	cust[value + 1] = '\0';
	cout << "1: " <<arraySize << endl;
	for(i; i <= len; i++){
		if(savings[i].accountNumber == NULL){
			strcpy(savings[i].accountNumber, anum);
			savings[i].balance = bal;
			strcpy(savings[i].customer, cust);
			size++;
			cout << "2: " << arraySize << endl;
		}
		else{
			cout << "3: " << arraySize << endl;
			while(i < arraySize){
				strcpy(temp[i].accountNumber, savings[i].accountNumber);
				temp[i].balance = savings[i].balance;
				strcpy(temp[i].customer, savings[i].customer);
				i++;
			}
				
			delete [] savings;

			savings = new Account[arraySize + 10];

			i = 0;
			for(i; i < len; i++){
				strcpy(savings[i].accountNumber, temp[i].accountNumber);
				savings[i].balance = temp[i].balance;
				strcpy(savings[i].customer, temp[i].customer);
			}
			delete [] temp;

			strcpy(savings[i].accountNumber, anum);
			savings[i].balance = bal;
			strcpy(savings[i].customer, cust);

			size++;
		}
	}
	arraySize = arraySize + 10;
	cout << arraySize << endl;
	return data;
}

[kenji]

I made a mistake with the above code, I had changed a lot of the code but copied the wrong file.

Basically what I have to do is search an dynamically allocated array to find an empty space and if I find one I copy data to it, otherwise I need to create a temporary array and copy data that and increase the array length of the dynamically allocated array called savings. Once created I copy the data back to savings and delete the temporary array.

I need to break out of the for loop once I run the else statement, I am stuck in the loop until the terminating condition is met.

const char* Bank::operator+=(const char data[]){
	int i = 0;
	const int len = arraySize;
	Account *temp;
	temp = NULL;
	temp = new Account[len];
	int bal;
	char anum[15 + 1];
	char cust[315 + 1];
	
	sscanf(data,"%15[^,],%d,%315[^;];", anum, &bal, cust);

	int value = strlen(cust);
	cust[value] = ';';
	cust[value + 1] = '\0';
	
	for(i; i <= len; i++){
		if(savings[i].accountNumber == NULL){
			strcpy(savings[i].accountNumber, anum);
			savings[i].balance = bal;
			strcpy(savings[i].customer, cust);
			size++;
			cout << "2: " << arraySize << endl;
		}
		else{
			cout << "3: " << arraySize << endl;
			while(i < arraySize){
				strcpy(temp[i].accountNumber, savings[i].accountNumber);
				temp[i].balance = savings[i].balance;
				strcpy(temp[i].customer, savings[i].customer);
				i++;
			}
				
			delete [] savings;

			savings = new Account[arraySize + 10];

			i = 0;
			for(i; i < len; i++){
				strcpy(savings[i].accountNumber, temp[i].accountNumber);
				savings[i].balance = temp[i].balance;
				strcpy(savings[i].customer, temp[i].customer);
			}
			delete [] temp;

			strcpy(savings[i].accountNumber, anum);
			savings[i].balance = bal;
			strcpy(savings[i].customer, cust);

			size++;
		}
	}
	arraySize = arraySize + 10;
	cout << arraySize << endl;
	return data;
}

[StuXYZ]

This is not much better.

(a) size++; (Line 22) I think that should be arraySize.

(b) You have a for loop with i, (line 17) . Then you do not allow the loop to continue but have a while(i<arraySize) . Oh yes and the for loop is actually i<=arraySize because of the indirection of setting len=arraySize.
Then just incase this isn't a winner for the obstaficated c++ contest, you have another while further down using the same variable .

This is 100% a mess.

Then you have a loop in a loop using the same index variable.

Summary:

(A) Bin this code -- This will save you time.
(B) Do the job in stages:
(i) Find the first savings which has a null accountNumber.
call that variable Index.

(ii) Then have two pieces of code

If (Index<arraySize)
          {
              copy new stuff in to place. 
          }
      if (index==arraySize)
         {
             increase arraySize by 10
             copy new stuff into place
         }

Looking at the above you can easily see that it should be

If (Index==arraySize)
           {
               increase arraySize by 10
           }
       copy new stuff into place

Job done, time for something else. [Writing your unit test]

[kenji]

stuXYZ thanks for your help Iv managed to recode and get the function to work.

[StuXYZ]

Well let us see the answer then !!!!

[kenji]

Here you go.

const char* Bank::operator+=(const char data[]){
	int i = 0;
	const int len = arraySize;
	Account *temp;
	temp = NULL;
	temp = new Account[len];
	int bal;
	char anum[15 + 1];
	char cust[315 + 1];
	
	sscanf(data,"%15[^,],%d,%315[^;];", anum, &bal, cust);

	int value = strlen(cust);
	cust[value] = ';';
	cust[value + 1] = '\0';
	cout << "1: " <<arraySize << endl;

	if(savings[i].accountNumber != NULL){
		i++;
	}

	if(i < arraySize){
		strcpy(savings[i].accountNumber, anum);
		savings[i].balance = bal;
		strcpy(savings[i].customer, cust);
		size++;
		//cout << "2: " << arraySize << endl;
	}

	if(i > arraySize){
		while(int j = 0 < arraySize){
			strcpy(temp[j].accountNumber, savings[j].accountNumber);
			temp[j].balance = savings[j].balance;
			strcpy(temp[j].customer, savings[j].customer);
			i++;			
		}

		delete [] savings;
		savings = new Account[arraySize + 10];
		cout << arraySize << endl;

		int k = 0;
		while(k <= arraySize + 10){
			strcpy(savings[i].accountNumber, temp[i].accountNumber);
			savings[i].balance = temp[i].balance;
			strcpy(savings[i].customer, temp[i].customer);
		}
		delete [] temp;

		strcpy(savings[i].accountNumber, anum);
		savings[i].balance = bal;
		strcpy(savings[i].customer, cust);
		size++;
	}

	arraySize = arraySize + 10;
	cout << arraySize << endl;
	return data;
}

[StuXYZ]

Oh dear.... This doesn't look very tested....

Line 18/19 Do you really want an if.... I think this is required

for(;i<arraySize && savings[i].accountNumber!=NULL;i++) ;

and you always increase the arraySize regardless...

and you do absolutely nothing if i==arraySize....

hmm.....

oh and classic errors on the both while loops since you never increase j or k.

Overall a slightly better attempt, but you didn't go for my second psuedo code version
which is a shame since you have code repeat with different errors and hence twice the debugging to do.

Anyway, enjoy the debugging ....