[Hero26]

Hello and thank you for looking at my problem.
Firstly, I've taken this out of a larger program to try reduce the complexity because I'm only really having a problem with the 'DeleteStudent' function. Therefore, my apologies if this looks messy.
I need the user to be able to delete one of the structures in the array of structures (each structure representing one student) and then the program should remove the wasted empty memory from the array. This is my ultimate goal for this function, but at the moment it displays garbage in the deleted structure, or all the structures.

#include <stdio.h>
#include <stdlib.h>
#include <conio.h>
#include <math.h>
#include <ctype.h>
#include <string.h>
#define TRUE 1
#define FALSE 0
#define NOT_FOUND -1

struct STUDENT* DeleteStudent(struct STUDENT* StudentPointer, int NumStudentsInMemory);
int FindStudent(struct STUDENT *StudentPtr, int NumberOfStudents);
void ClearArray(struct STUDENT Array[], int ArraySize, int MaxNameLength);
struct STUDENT* GetUserToAddStudents(struct STUDENT* StudentPtr, int* StudentsCurrentlyInMemory);
struct STUDENT* AddStudent(struct STUDENT *StudentPtr, int* StudentsCurrentlyInMemory);
void DisplayAllStudents(struct STUDENT *StudentPtr, int NumberOfStudents);

char GetValidatedYesOrNo();
int GetValidatedInteger(int Min, int Max);
char* Getvalidatedstring(char StringName[], int Minlen, int Maxlen);
void Pause();

struct STUDENT {
	char Name[10];
	int Mark;
};

int main()
{
	struct STUDENT *StudentPointer = NULL;
	int NumStudentsInMemory = 0;
	StudentPointer = GetUserToAddStudents(StudentPointer, &NumStudentsInMemory);
	DeleteStudent(StudentPointer, NumStudentsInMemory);
	DisplayAllStudents(StudentPointer, NumStudentsInMemory);
	Pause();
	return 0;
}

struct STUDENT* DeleteStudent(struct STUDENT* StudentPointer, int NumStudentsInMemory)
{
	int a = 0, Placeholder = 0;
	struct STUDENT *StudentPtr_temp = NULL;
	a = FindStudent(StudentPointer, NumStudentsInMemory);

	for(Placeholder = a; Placeholder < NumStudentsInMemory; Placeholder++)
	{
		StudentPointer[Placeholder] = StudentPointer[(Placeholder + 1)];
	}

	StudentPtr_temp = (struct STUDENT*)malloc(sizeof(struct STUDENT) * (NumStudentsInMemory - 1) );
	ClearArray(StudentPtr_temp, (NumStudentsInMemory - 1), 10);

	for(Placeholder = 0; Placeholder < NumStudentsInMemory; Placeholder++)
	{
		if(Placeholder != a)
		{
			StudentPtr_temp[Placeholder] = StudentPointer[Placeholder];
		}
	}

	if (StudentPointer != NULL)
	{
		free(StudentPointer);
	}

	StudentPointer = StudentPtr_temp;
	StudentPtr_temp = NULL;

	return StudentPointer;
}

int FindStudent(struct STUDENT *StudentPtr, int NumberOfStudents)
{
	int i = 0, Index = NOT_FOUND;
	char Name[11];

	printf("\nEnter student's name to search for: ");
	Getvalidatedstring(Name, 1, 10);
	
	for(i = 0; i < NumberOfStudents; i++)
	{
		/* Check each book against search-string */
		if(stricmp(StudentPtr[i].Name, Name) == 0)
		{
			Index = i;
			break;
		}
	}
	
	return(Index);
}

void ClearArray(struct STUDENT Array[], int ArraySize, int MaxNameLength)
{
	int Index = 0;
	for(Index = 0; Index < ArraySize; Index ++)
	{
		memset(Array[Index].Name, ' ', MaxNameLength);
		strcpy(Array[Index].Name, "");
		Array[Index].Mark = 0;
	}
}

struct STUDENT* GetUserToAddStudents(struct STUDENT* StudentPtr, int* StudentsCurrentlyInMemory)
{
	char Continue = 'Y';

	printf("\nAdd a student\n");

	do
	{
		StudentPtr = AddStudent(StudentPtr, StudentsCurrentlyInMemory);
		(*StudentsCurrentlyInMemory)++;
		printf("\nDo you want to add another student? (Y/N)");
		Continue = GetValidatedYesOrNo();

	}while(toupper(Continue) == 'Y');

	return StudentPtr;
}

struct STUDENT* AddStudent(struct STUDENT *StudentPtr, int* StudentsCurrentlyInMemory)
{ 	
	int Count = 0;
	struct STUDENT *StudentPtr_temp = NULL;

	//allocate enough memory to make a copy of student records currently in memory plus space for 1 extra to be added
	StudentPtr_temp = (struct STUDENT*)malloc(sizeof(struct STUDENT) * (*StudentsCurrentlyInMemory + 1) );

	ClearArray(StudentPtr_temp, *StudentsCurrentlyInMemory + 1, 10);//clear any garbage in newly allocated memory

	//copy student records currently in memory into newly allocated memory
	for(Count = 0; Count < *StudentsCurrentlyInMemory; Count++)
	{
		StudentPtr_temp[Count] = StudentPtr[Count];
	}

	//get user to enter values for new student record which will be added to the last element of the newly allocated memory
	printf(" Student's Name: ");
	Getvalidatedstring(StudentPtr_temp[Count].Name, 1, 10);

	printf(" Student's Mark: ");
	StudentPtr_temp[Count].Mark = GetValidatedInteger(0, 100);

	if (StudentPtr != NULL)
	{
		free(StudentPtr);//free up redundant memory - *very important* - prevents memory leaks!!
	}

	StudentPtr = StudentPtr_temp;//now make StudentPtr point to the newly created memory
	StudentPtr_temp = NULL;//set StudentPtr_temp to NULL ready for the next time

	return StudentPtr;
}

void DisplayAllStudents(struct STUDENT *StudentPtr, int NumberOfStudents)
{
	int Count = 0;

	printf("\n\nAll records currently in memory\n---------------------------\n%10s %s\n", "Name", "Mark");

	for(Count = 0; Count < NumberOfStudents; Count++)
	{
		printf("%10s %d\n", StudentPtr[Count].Name, StudentPtr[Count].Mark);
	}
}

char GetValidatedYesOrNo()
{
	int ItemRead = FALSE;
	char Input = '\0';
	int Valid = FALSE; /* MUST be initialised to FALSE */

	do
	{
		/* Read. NB scanf() returns the number of items successfully read */
		ItemRead = scanf("%c", &Input);
		fflush(stdin);

		Input = toupper(Input);

		if(ItemRead == FALSE)
		{
			printf("Sorry, something went wrong with scanf. Please try again: ");
		}
		/* Validate - 'Y' or 'N'? */
		else if( (Input != 'Y') && (Input != 'N') )
		{
			printf("Invalid - must be either 'Y' or 'N'! Please try again: ");
		}
		else
		{
			Valid = TRUE;
		}
	} while(Valid == FALSE);

	return(Input);
}

/* Gets user-inputted integer value. Validated for data type & value (Min->Max). */
int GetValidatedInteger(int Min, int Max)
{
	int ItemRead = FALSE;
	double Input = 0; /* NB Input read into a FLOAT */
	int Valid = FALSE; /* MUST be initialised to FALSE */

	do
	{
		/* Read. NB scanf() returns the number of items successfully read */
		ItemRead = scanf("%lf", &Input);
		fflush(stdin);

		/*Validate - Non-numeric? (In which case scanf() will not have read in anything)*/
		if(ItemRead == FALSE)
		{
			printf("Invalid - can only be a number (not a letter, a string or a symbol)");
		}
		/* Validate - Not whole number? */
		else if(Input != (int)Input) /* NB (int)Input truncates a float value */
		{
			printf("Invalid - must be whole number! Please try again (%d-%d): ", Min, Max);
		}
		/* Validate - Invalid value? (ie Not in range Min->Max) */
		else if((Input < Min) || (Input > Max))
		{
			printf("Invalid - out-of-range! Must be a number between %d & %d! Please try again: ", Min, Max);
		}
		else
		{
			Valid = TRUE;
		}
	} while(Valid == FALSE);

	return((int)Input); /* NB Cast (store) as int before returning */
}

/* Note: Validated, user-inputted array passed back twice:
			1. as reference parameter, and
			2. as a return value (Note “pointer” syntax "char*"). 
		Reason - To offer calling code maximum flexibility of use. */
char* Getvalidatedstring(char StringName[], int Minlen, int Maxlen)
{
    
    char Input[200]; /* Very big to avoid array over-run. */
	
	do
	{
        
		gets(Input);

		if((strlen(Input) < Minlen) || (strlen(Input) > Maxlen))
		{
			printf("Invalid! Please try again (Length %d-%d): ", Minlen, Maxlen);
		}
	} while((strlen(Input) < Minlen) || (strlen(Input) > Maxlen));

	/* Using string handling library function (strcpy()) */
	strcpy(StringName, Input);/* 1. Populate reference parameter */
	printf("%s\n", StringName);
	return(StringName);/* 2. Return value */
}

void Pause()
{
	printf("\n\nPress enter to exit.\n");
	getch();
}

[CTphpnwb]

When I have trouble with a larger code set I find it easer to create a small example set that demonstrates the problem. Usually that will also show the solution.

#include <iostream>

struct example {
	int x, y;
	example(int xx, int yy) {
		x = xx;
		y = yy;
	}
	
	void show() {
		std::cout << x << "\t" << y << std::endl;
	}
	
};

void show_array(example *arr[], int size) {
	for (int i = 0; i < size; i++) {
		arr[i]->show();
	}
}

void erase_element(example *arr[], int size, int elem) {
	if (arr[elem] != NULL) {
		delete arr[elem];
		size--;
		for (int mv = elem; mv < size; mv++) {
			arr[mv] = arr[mv+1];
		}
	}
}

int main(int argc, const char * argv[])
{
	example *test[10];
	
	for (int i = 0; i < 10; i++) {
		test[i] = new example(i,i+100);
	}
	
	show_array(test, 10);
	erase_element(test, 10, 5);
	std::cout << "****\n";
	show_array(test, 9);
	
	return 0;
}

[Hero26]

Thank you for posting that. It is much clearer. However, I believe 'new' and 'delete' are for C++. I am working in C. If I use 'malloc' and 'free' in place of these then I get build errors. How should I change their useage?

[CTphpnwb]

Because I don't like it I'm not good with C, but this is close:

#include <iostream>

struct example {
	int x, y;
	
	example(int xx, int yy) {
		x = xx;
		y = yy;
	}
	
	void show() {
		std::cout << x << "\t" << y << std::endl;
	}
	
};

void show_array(example *arr[], int size) {
	for (int i = 0; i < size; i++) {
		arr[i]->show();
	}
}

void erase_element(example *arr[], int size, int elem) {
	if (arr[elem] != NULL) {
		free(arr[elem]);
		size--;
		for (int mv = elem; mv < size; mv++) {
			arr[mv] = arr[mv+1];
		}
	}
}

int main(int argc, const char * argv[])
{
	example *test[10];
	
	for (int i = 0; i < 10; i++) {
		test[i] = (example *) malloc(sizeof(example));
		test[i]->x = i;
		test[i]->y = i+100;
	}
	
	show_array(test, 10);
	erase_element(test, 10, 5);
	std::cout << "****\n";
	show_array(test, 9);
	
	return 0;
}

The struct is still more C++ than C, which is (I think) why the constructor isn't working, but you can see that the memory is being allocated and then freed up.

[jimblumberg]

Quote

The struct is still more C++ than C, which is (I think) why the constructor isn't working

Yes, C doesn't allow member functions, just data inside structures.

Here is a C example:

#include <stdio.h>
#include <stdlib.h>

typedef struct example {
   int x;
   int y;
} Example;

void initialize(Example *array, int size);
void show(Example element);
void show_array(Example *arr, int size);
void erase_element(Example **arr, int *size, int elem);
 
void initialize(Example *array, int size)
{
   for(int i = 0; i < size; ++i)
   {
      array[i].x = i + 1;
      array[i].y = i + 1;
   }
}
void show(Example element)
{
   printf("%d,%d", element.x, element.y);
}
 
void show_array(Example arr[], int size)
{
   for (int i = 0; i < size; i++)
   {
      show(arr[i]);
      printf("\t");
   }
}
 
void erase_element(Example **arr, int *size, int elem)
{
   for (int mv = elem; mv < *size - 1; mv++)
   {
      (*arr)[mv] = (*arr)[mv+1];
   }
   (*size)--;
   // Free the memory.
   *arr = realloc(*arr, sizeof(Example) * (*size));
}
 
int main(void)
{
   Example *test;
   int size = 10;
   test = malloc(size * sizeof(Example));
   initialize(test, size);
   show_array(test, size);
   erase_element(&test, &size, 5);
   printf("\n****\n");
   show_array(test, size);
   free(test);
   return 0;
} 

Jim

[Hero26]

Thank you for you example written in C. I'm afraid I'm still having trouble implementing this and understanding it. Your code looks good but I have tried building your code in my compiler and it didn't build. I'm not sure why.

Regarding the things I'm having trouble understanding: Firstly I can't see where you have actually initialised the structure as an array. Secondly is there are way to do this without using double pointers? I have not yet covered that topic. Thirdly, this is just a minor thing, my compiler doesn't let me initialise the int 'i' (or any other for that matter) within the brackets of the 'for' loop. It has to be in the body of the function. This was one of the reasons your code didn't build. Do you know why this works for you and not me?

I have tried to write my own version of your example in a way I understand. There are several problems with it that I would greatly appreciate your help in understanding: Firstly, malloc and realloc are not accepting the data type for my structure, I tried forcing malloc to take that type of data but that didn't work either. Secondly I tried followed your method of initialising the structure within my own understanding, and because I wanted to understand your example more clearly I resisted putting in the square brackets while initialising the structure. But in my example it hasn't come out as an array as it has in yours.

I didn't think allowing the user to delete something would turn out to be this difficult.

#include <stdio.h>
#include <stdlib.h>

void Input(struct EXAMPLE *Example, int size);
void Display(struct EXAMPLE *Example, int size);
void Erase_Element(struct EXAMPLE **Example, int *size, int For_Deletion);
void Pause();

struct EXAMPLE
{
	int x;
};

int main()
{
	struct EXAMPLE *Example;
	int size = 10;
	Example = malloc(size * sizeof(struct EXAMPLE));
	Input(Example, size);
	Display(Example, size);
	Erase_Element(&Example, &size, 5/*Would be user selected element number here.*/);
	Display(Example, size);
	free(Example);
	Pause();
	return 0;
}

void Input(struct EXAMPLE Example, int size)
{
	int i = 0;
	for(i = 0; i < size; ++i)
	{
		Example[i].x = i + 1;
	}
}

void Display(struct EXAMPLE Example, int size)
{
	int i = 0;
	for(i = 0; i < size; i++)
	{
		printf("%d\n", Example[i].x);
	}
}

void Erase_Element(struct EXAMPLE *Example, int *size, int For_Deletion)
{
	int move = 0;
	for(move = For_Deletion; move < *size - 1; move++)
	{
		(Example)[move] = (Example)[move = 1];
	}
	(*size)--;
	Example = realloc(Example, sizeof(Example) * (*size));
}

void Pause()
{
	printf("\n\nPress enter to exit.\n");
	getchar();
}

[jimblumberg]

Quote

our code looks good but I have tried building your code in my compiler and it didn't build. I'm not sure why.

Quote

my compiler doesn't let me initialise the int 'i' (or any other for that matter) within the brackets of the 'for' loop. It has to be in the body of the function. This was one of the reasons your code didn't build. Do you know why this works for you and not me?

Post the error messages. The code I supplied requires a C99 or C11 compliant compiler.

Quote

Firstly I can't see where you have actually initialised the structure as an array.

I created the array using malloc. That is one of the purposes of malloc, to create an array at compile time. I initialized the array in my initialize() function.

Quote

Secondly is there are way to do this without using double pointers?

The double pointer is required in the erase_element() function because of the realloc() call. The realloc() function can change the address of the memory block, therefore you need to pass the actual pointer, hence the double pointer. Without this double pointer you pass a copy of the pointer (pass by value). If you remove the requirement to free up the memory then you can remove the realloc() call and use single pointers.

One of the problems with your code is in the following snippet:

void Input(struct EXAMPLE *Example, int size);
void Display(struct EXAMPLE *Example, int size);
void Erase_Element(struct EXAMPLE **Example, int *size, int For_Deletion);
void Pause();

struct EXAMPLE
{
	int x;
};

You need to move the structure declaration to before the function prototypes because the prototypes need to know about this structure.

Quote

Firstly, malloc and realloc are not accepting the data type for my structure, I tried forcing malloc to take that type of data but that didn't work either.

Why doesn't malloc accept the data type?

You need to post your error messages.

Quote

I didn't think allowing the user to delete something would turn out to be this difficult.

The difficulty you're having is not deleting the act element, you seem to understand the moving down of the following elements. The problem you seem to be having is with freeing the memory for this released element.

Your example is close, you have a couple of simple errors (an = instead of a +) and a few missing *. Post your error messages then I can help you better understand what your compiler is telling you.

Jim

[Hero26]

Hello again. I have made progress since last we spoke and have the program working up to a point. The program correctly deletes, but only where there is only 1 field in the structure. I have no idea why.
Here is my example program indicating what it is supposed to do:

#include <stdio.h>
#include <stdlib.h>

void Input(struct EXAMPLE Example[], int size);
void Display(struct EXAMPLE Example[], int size);
struct EXAMPLE* Erase_Element(struct EXAMPLE Example[], int *size, int For_Deletion);
void Pause();

struct EXAMPLE
{
	int x;
};

int main()
{
	struct EXAMPLE* ExamplePtr;
	int size = 10;
	int idxToDelete = 5;
	ExamplePtr = (struct EXAMPLE *)malloc(size * sizeof(struct EXAMPLE));
	Input(ExamplePtr, size);
	Display(ExamplePtr, size);
	printf("Deleteing the %dth element...\n", idxToDelete + 1);
	ExamplePtr = Erase_Element(ExamplePtr, &size, idxToDelete/*Would be user selected element number here.*/);
	Display(ExamplePtr, size);
	free(ExamplePtr);
	Pause();
	return 0;
}

void Input(struct EXAMPLE Example[], int size)
{
	int i = 0;
	for(i = 0; i < size; ++i)
	{
		Example[i].x = i + 1;
	}
}

void Display(struct EXAMPLE Example[], int size)
{
	int i = 0;
	for(i = 0; i < size; i++)
	{
		printf("Element %d: %d\n", i+1, Example[i].x);
	}
}

struct EXAMPLE * Erase_Element(struct EXAMPLE Example[], int *size, int For_Deletion)
{
	int move = 0;
	for(move = For_Deletion; move < *size - 1; move++)
	{
		Example[move] = Example[move + 1];
	}
	(*size)--;
	Example = (struct EXAMPLE *)realloc(Example, sizeof(Example) * (*size));
	return Example;
}

void Pause()
{
	printf("\n\nPress enter to exit.\n");
	getchar();
}

And now here is slightly altered example program with multiple fields in the structure and that only outputs garbage:

#include <stdio.h>
#include <stdlib.h>

//void Input(struct EXAMPLE *Example[], int size);
//void Display(struct EXAMPLE *Example[], int size);
//void Erase_Element(struct EXAMPLE *Example[], int *size, int For_Deletion);
void Input(struct EXAMPLE Example[], int size);
void Display(struct EXAMPLE Example[], int size);
struct EXAMPLE* Erase_Element(struct EXAMPLE Example[], int *size, int For_Deletion);
void Pause();

struct EXAMPLE
{
	int x;
	char y;
};

int main()
{
	//struct EXAMPLE *Example[10];
	struct EXAMPLE* ExamplePtr;
	int size = 10;
	int idxToDelete = 5;
	//*Example = (struct EXAMPLE *)malloc(size * sizeof(struct EXAMPLE));
	ExamplePtr = (struct EXAMPLE *)malloc(size * sizeof(struct EXAMPLE));
	Input(ExamplePtr, size);
	Display(ExamplePtr, size);
	printf("Deleteing the %dth element...\n", idxToDelete + 1);
	ExamplePtr = Erase_Element(ExamplePtr, &size, idxToDelete/*Would be user selected element number here.*/);
	Display(ExamplePtr, size);
	free(ExamplePtr);
	Pause();
	return 0;
}

void Input(struct EXAMPLE Example[], int size)
{
	int i = 0;
	char a = 'a';
	for(i = 0; i < size; ++i)
	{
		Example[i].x = i + 1;
		Example[i].y = a;
		a++;
	}
}

void Display(struct EXAMPLE Example[], int size)
{
	int i = 0;
	for(i = 0; i < size; i++)
	{
		printf("Element %d: %d\n", i+1, Example[i].x);
		printf("Element %d: %c\n", i+1, Example[i].y);
	}
}

//void Erase_Element(struct EXAMPLE *Example[], int *size, int For_Deletion)
struct EXAMPLE * Erase_Element(struct EXAMPLE Example[], int *size, int For_Deletion)
{
	int move = 0;
	for(move = For_Deletion; move < *size - 1; move++)
	{
		//Example[move] = Example[move = 1];
		Example[move] = Example[move + 1];
	}
	(*size)--;
	//*Example = (struct EXAMPLE *)realloc(Example, sizeof(Example) * (*size));
	Example = (struct EXAMPLE *)realloc(Example, sizeof(Example) * (*size));

	return Example;
}

void Pause()
{
	printf("\n\nPress enter to exit.\n");
	getchar();
}

[snoopy11]

Hi,

What problems are you encountering as the above seems to work for me ?

Snoopy.

[Hero26]

snoopy11, on 10 July 2013 - 02:36 PM, said:

Hi,

What problems are you encountering as the above seems to work for me ?

Snoopy.

The second code I posted doesn't delete the function properly. It doesn't keep its variable, it just prints garbage instead. I copied what I posted in this thread and tried building that and it is definitely broken.

[snoopy11]

I get

Element 1:1
Element 1:a
Element 2:2
Element 2:b
Element 3:3
Element 3:c
Element 4:4
Element 4:d
Element 5:5
Element 5:e
Element 6:6
Element 6:f
Element 7:7
Element 7:g
Element 8:8
Element 8:h
Element 9:9
Element 9:i
Element 10:10
Element 10:j

then it says deleting element 6 which it does

then it prints out 1 to 9 elements
and at element 6 is now 7,g

where is the garbage ?

Snoopy.

[Hero26]

This is a screen shot of the output when I run the program. Please see attached.
I am running Visual Studio 2010 Express if that makes any difference.

Attached image(s)

• 

[snoopy11]

I was running my new installation of code::blocks with a 64bit MinGW compiler

I will try it out on MSVC

Snoopy.

[snoopy11]

Yeah I get the same on MSVC wonder why that is....


need to dig around more

Snoopy.

[Hero26]

snoopy11, on 10 July 2013 - 03:13 PM, said:

Yeah I get the same on MSVC wonder why that is....


need to dig around more

Snoopy.

I appreciate you taking a look.