Need help with simple c++ function

[Ricochet]

Basically what this function does is compact an array so that, if there are empty spaces in the structure, the following items in the struct are moved up, and the empty spaces are added on at the end. At first, I made the function without using a temp array, but I had a problem, so I tried it this way, and I have the same problem. Basically, when I try to print the array, I get an error and the program stops. I think it's because the NULL pointer is not being assigned properly at the end, and there's just random data for those pointers. But I can't figure out what I'm doing wrong. Any ideas?

int compact (holder & info)
{
	holder temp;
	temp.count = info.count;
	temp.data=new person*[temp.count];
	int j = 0;
	for(int k = 0; k<= temp.count; k++)
	{
			temp.data[k] = NULL;
	}

	for(int i = 0; i<=info.count; i++)
	{
		if(info.data[i] != NULL)
		{
			temp.data[j] = info.data[i];
			j++;
		}
	}
return 1;
}

[JaWiB]

I assume holder is a class or a struct, so it would be helpful to see the declarations...Some of the lines look suspicious:

temp.data=new person*[temp.count];

What is data exactly? It looks like you are trying to allocate space for a two-dimensional array, but this won't be enough to do so...If you are allocated a one-dimensional array get rid of the '*' in that:

temp.data=new person[temp.count];

Also, an easier way to zero out the contents of that array would be to use the memset() function.

Lastly, you will go out of bounds in your loops:

for(int k = 0; k<= temp.count; k++)

Will loop up to temp.count; unfortunately arrays start with a 0 subscript (temp.data[0]) so you should loop like so:

for (int k=0; k<temp.count; k++)

[axon]

which array wre you trying to print from? info.data or temp.data ; in your code the only array changed is temp.data. After that for loop where you are lokking for !=NULL, after that has gone through you could set the last element of temp.data to NULL, and then to print it just go up to the NULL. You have the position, which is j.

In other words, after the second for loop has executed set temp.data[j] to NULL...

Also after you're done with temp set those value in the array that you've passed through and delete it.

This whole process though could e done much easier with a recursive call.

Also, what are you trying to the as a whole...maybe linked list would be a better idea.

[Ricochet]

Sorry, maybe this will help:

struct person {
	char name[20];
	int id;
};

struct holder {
	int count;
	person ** data;
};

data is the pointer to the person struct. That's the same code I used to allocate space for the original array.

I didn't know about the memset function. Thanks.

Also, thanks for pointing out the bit in the loop. I thought I fixed that once before, but it looks like I didn't.



I got the problem sorted. Looks like it was just the <= in the for loop. What a pain. :-X Thanks for your help though.

[Sebastiani]

Also, you need to re-adjust info.count afterward to reflect the new size. Obviously, if you're using that variable to keep track of the maximum capacity of the array, you'll need another variable added to the class. Here's a simple way to do it without using a temporary variable:

int
 compact(holder & info)
{ 
 int j = 0;

   for(int i = 0; i < info.count; i++)
  {
      if(info.data[i] != NULL)
     {
         info.data[j++] = info.data[i];
     }
  }

 return info.count = j;
}

[Ricochet]

Well, the size of the array isn't changing, as I'm just shifting the data around.

This is what I had before w/o the temp array:

int compact(holder & info)
{ 
	int ptr = NULL;
	for(int i = 0; i < info.count; i++)
	{
		if(info.data[i]==NULL && ptr == NULL)
		{
			ptr = i;
		}
		else if(info.data != NULL && ptr !=NULL)
		{
			info.data[ptr] = info.data[i];
			info.data[i] = NULL;
			if(info.data[(ptr+1)]==info.data[i]==NULL)
				ptr++;
			else
				ptr=i;
		}
	}
	return 1;
}

The problem is that I had to call the function more than once if I had more than one NULL pointer in the array. I couldn't see why, so I just cheated and used a temp variable.

Also, the reason I didn't use a linked list or recursion, is because this is for a class at school and at the time of the assignment we hadn't learned recursion or linked lists yet. The teacher is one of those that allows us to turn in stuff whenever we want. I've got myself into trouble now, as I have 4 weeks left in the semester to do all the homework.

[axon]

Originally posted by Ricochet
Also, the reason I didn't use a linked list or recursion, is because this is for a class at school and at the time of the assignment we hadn't learned recursion or linked lists yet. The teacher is one of those that allows us to turn in stuff whenever we want. I've got myself into trouble now, as I have 4 weeks left in the semester to do all the homework.

procrastination is the enemy of students all over the world....

[Felix]

Maybe make the for loop one bit longer?
Or try using >> or >>> dunno how they work though lol ^^