Function not working properly please help

[mikecolistro]

Hi i'm in a university class, and i'm working on a little assignment that is supposed to have an int array and have assorted functions to do things to the array currently i have two of the four functions working and i'm having trouble with the function that is supposed to find the min value and the one that finds the max, once you solve one you most likely solve the other thank you.

#include <stdio.h>
#include <stdlib.h>
#define STUDENTS 3
#define EXAMS 4
#define MIN_NUM 999999999999
int printArray(int *arry1,int n, int k);
int findMin(int *arry1,int n, int k);
int findAVG(int *arry1,int n, int k);

int (*ptr1)(int *,int,int);
int (*ptr2)(int *,int,int);
int (*ptr4)(int *,int,int);

int main(){
	int studentGrades[STUDENTS][EXAMS]= {{ 77 , 68 , 86 , 73 },{ 96 , 87 , 89 , 78 },{ 70 , 90 , 86 , 81 } } ;
	

	ptr1 = &printArray;
	ptr2 = &findMin;
	ptr4 = &findAVG;
	int choice;
	printf("Enter in a value to select the following options from the menu\n0-Print the array of grades\n1-Find the minimum grade\n2-Find the maximum grade\n3-Print the average on all tests for each student\n4-End program\n");  
	scanf("%d", &choice);

	if(choice == 0){
		
		ptr1(*studentGrades,STUDENTS,EXAMS);
	}
	else if(choice == 1){
		ptr2(*studentGrades,STUDENTS,EXAMS);
	}
	else if(choice == 2){
	}
	else if(choice == 3){
		ptr4(*studentGrades,STUDENTS,EXAMS);
	}
	else if(choice == 4){
		exit(1);
	}

}


int printArray(int* array, int num_rows, int num_cols)
 {
    int i, j;
  
    for (i=0; i < num_rows; i++)
    {
      for(j=0; j < num_cols; j++)
        printf("%d ", *(array + (num_rows * i) + j));
      printf("\n");
    }
	return 0;
  }


int findMin(int* array, int num_rows, int num_cols){
	int min = MIN_NUM;
	for(int i = 0; i < num_rows; i++){
		for(int j = 0; j < num_cols; j++){
			if(min < *(array + (num_rows * i) + j))
				min = *(array + (num_rows * i) + j);
		}
	}
	return 0;
	printf("The minimum grade in the array is %d", min);
}
/*
void findMax(char *arry1[][], int n, int k){
	int max = arry1[0][0];
	for(int i = 0; i < n; i++){
		for(int j = 0; j < k; j++){
			if(max < arry1[i][j])
				max = arry1[i][j];
		}
	}
	printf("The maximum grade in the array is %d", max);


}
*/
int findAVG(int* array, int num_rows, int num_cols){
	int avg, temp = 0;
	for(int i = 0; i < num_rows; i++){
		for(int j = 0; j < num_cols; j++){
			temp = temp + *(array + (num_rows * i) + j);
		}
	}
	avg = temp / (num_rows * num_cols);
	printf("The average grade in the array is %d", avg);
	return 0;
}

[frogboy77]

I would suggest posting less code and a specific question about what you are looking for.

[mike_2000_17]

You made a typical logic error. Your algorithms for min is inverted. That is, your min function will actually return the maximum value. Just change the name and you have the max function done. The little bit of logic confusion is that when you are looking for the minimum, you should first set your variable to the maximum possible number (MAX_NUM or some other large number) and then replace it with every number in the array that is smaller than the one you have stored currently. You have done the opposite in your min function, you first set to MIN_NUM and then replace its value by every element that is bigger than the current one... at the end, that will get you the maximum value in the array.

BTW, don't let your ego take a hit because of this silly mistake, this is actually a surprisingly easy-to-make error (I find myself doing it too, once in a while).

[ravenous]

I tend not to use predefined MAX_NUM or MIN_NUM values when finding the min and max of arrays. Instead, I tend to initialise the min or max to the first value of the array and then loop over the remainder of the array. So, in your case a findMin() function would look like:

int findMin(int *in, int rows, int cols){
   int *min = *in;
   int *end = in + rows*cols;

   for(int *p = in + 1; p != end; ++p)
      min = *p < min ? *p : min;

   return min;
}

Also, I have only used one loop, not two nested loops, since I have a feeling this is faster, and the function doesn't need to care about the row/column structure to find the min/max.

To add to the final thing that mike_2000_17 said, I make a related mistake sometimes, where I want to make both a findMin() and a findMax() . I write the first and then copy & paste then change the details to make the second, except that I fail to change all of the details correctly!

[vinitmittal2008]

well... you just passing a 2D array as a 1D array. then just use this code

int findMin(int* array, int num_rows, int num_cols){

	int min = *array; // or can use array[0]
	int i;
	for( i = 1; i < num_rows * num_cols; i++){
		  if(min > *(array + i))
		       min = *(array + i);


	}
	return min;
}

[ravenous]

well... you just passing a 2D array as a 1D array. then just use this code

int findMin(int* array, int num_rows, int num_cols){

	int min = *array; // or can use array[0]
	int i;
	for( i = 1; i < num_rows * num_cols; i++){
		  if(min > *(array + i))
		       min = *(array + i);


	}
	return min;
}

Er... that's what I just said.

Actually I have something extra to add as well :) I thought about it a bit more and there are two things that you should do to make this kind of function safer and better. Firstly, exercise the principle of least access and secondly allow error checking. So I would re-write this to be something more like:

int findMin(const int * const in, const int size, int &min){
   if(size <= 0)
      return ERROR_VALUE;

   min = *in;
   const int * const end = in + size;
 
   /* Make the pointer in the loop point to a const int, principle of least privilege */
   for(const int *p = (const int *)(in + 1); p != end; ++p)
      min = *p < min ? *p : min;
 
   return SUCCESS;
}

Here, I'm declaring the arguments to the function const if they don't need to change the values of the variables that are passed to the function. The most important thing to declare const is the name of the array that is passed to the function. I've used const int * const in here, that is a pointer to a const int variable where the pointer variable itself is const . This means that it's a bit harder to accidentally change the values in the array (since there should be no need to do this in this function). It doesn't make it impossible, but it's harder to do it by accident. You could still do something like:

int *x = (int *)(in);
*x = 100;

but, you can see it's more difficult to do. The second const is the important one, since if you were to do something like ++in; then this would cause a compilation error in this case, but in the original case would be allowed. So, if this was allowed, for the rest of the program after you call findMin() then in will not be pointing to the start of your array, but the second element! This is obviously going to cause a problem.

I've also passed the variable that min will be stored in by reference and used the return value of the function for error-checking. This allows us to check that the size given for the array is a sensible number (although, not whether the size actually corresponds to the size of the array that's passed. That has to be left up to the user, unless you create a special structure that contains the array and a variable that automatically keeps track of the size).

So, now you'd use the function like this:

#include <stdio.h>

/* I use these definitions to aid readability */
#define SUCCESS 0
#define ERROR_VALUE 1

/* returns value for error-checking */
int findMin(const int * const in, const int size, int &min);

int main(){
    /* Make an array of test data */
    int array[] = { 6, 3, 2, 7, 3, 5, 1, 6, 3, 8, 4, 8, 0, 3, 7, 3, 6 };
    int rows = 4;
    int cols = 4;
    int minimum;

    /* find the minimum and check if eveything went OK */
    if(findMin(array,rows*cols,minimum)){
        printf("Error!\n");
        return ERROR_VALUE;
    }
    else    /* Print out the result if everything was OK */
        printf("minimum = %d\n",minimum);

    /* Woop! */
    return SUCCESS;
}

int findMin(const int * const in, const int size, int &min){
   if(size <= 0)
      return ERROR_VALUE;

   min = *in;
   const int * const end = in + size;

   for(const int *p = (const int *)(in + 1); p != end; ++p)
      min = *p < min ? *p : min;

   return SUCCESS;
}

It might seem over complicated, but will probably help prevent errors if you (or someone else) come(s) back to the code in two years and tries to add something :)