Need help with simple c++ function

This is a discussion on Need help with simple c++ function within the C++ Programming forums, part of the General Programming Boards category; Basically what this function does is compact an array so that, if there are empty spaces in the structure, the ...

  1. #1
    Registered User
    Join Date
    Nov 2003
    Posts
    22

    Need help with simple c++ function

    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?

    Code:
    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;
    }

  2. #2
    carry on JaWiB's Avatar
    Join Date
    Feb 2003
    Location
    Seattle, WA
    Posts
    1,972
    I assume holder is a class or a struct, so it would be helpful to see the declarations...Some of the lines look suspicious:

    Code:
    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:
    Code:
    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:
    Code:
    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:
    Code:
    for (int k=0; k<temp.count; k++)
    "Think not but that I know these things; or think
    I know them not: not therefore am I short
    Of knowing what I ought."
    -John Milton, Paradise Regained (1671)

    "Work hard and it might happen."
    -XSquared

  3. #3
    Registered User axon's Avatar
    Join Date
    Feb 2003
    Location
    Mt. Prospect, IL
    Posts
    2,572
    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.

    some entropy with that sink? entropysink.com

    there are two cardinal sins from which all others spring: Impatience and Laziness. - franz kafka

  4. #4
    Registered User
    Join Date
    Nov 2003
    Posts
    22
    Sorry, maybe this will help:

    Code:
    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.
    Last edited by Ricochet; 11-12-2003 at 11:11 PM.

  5. #5
    Guest Sebastiani's Avatar
    Join Date
    Aug 2001
    Location
    Waterloo, Texas
    Posts
    5,600
    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:

    Code:
    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;
    }
    Code:
    if( numeric_limits< byte >::digits != bits_per_byte )
        error( "program requires bits_per_byte-bit bytes" );
    24bbs.cpp

  6. #6
    Registered User
    Join Date
    Nov 2003
    Posts
    22
    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:

    Code:
    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.
    Last edited by Ricochet; 11-13-2003 at 12:01 AM.

  7. #7
    Registered User axon's Avatar
    Join Date
    Feb 2003
    Location
    Mt. Prospect, IL
    Posts
    2,572
    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....

    some entropy with that sink? entropysink.com

    there are two cardinal sins from which all others spring: Impatience and Laziness. - franz kafka

  8. #8
    Registered User
    Join Date
    Nov 2003
    Posts
    168
    Maybe make the for loop one bit longer?
    Or try using >> or >>> dunno how they work though lol ^^
    -Felix
    Rots Soft
    If the facts don't fit the theory, change the facts.
    Albert Einstein (1879 - 1955)

Popular pages Recent additions subscribe to a feed

Similar Threads

  1. Replies: 28
    Last Post: 07-16-2006, 11:35 PM
  2. Change this program so it uses function??
    By stormfront in forum C Programming
    Replies: 8
    Last Post: 11-01-2005, 07:55 AM
  3. const at the end of a sub routine?
    By Kleid-0 in forum C++ Programming
    Replies: 14
    Last Post: 10-23-2005, 06:44 PM
  4. Binary Search Trees Part III
    By Prelude in forum A Brief History of Cprogramming.com
    Replies: 16
    Last Post: 10-02-2004, 03:00 PM
  5. I need help with passing pointers in function calls
    By vien_mti in forum C Programming
    Replies: 3
    Last Post: 04-24-2002, 10:00 AM

1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21