trouble sorting array of numbers

[mtatadotcom]

hello, i need help with this sub program, everything ele works fine except the subprogram that should compute the smallest and largest number in the array

#include<conio.h>
#include<stdio.h>
#define MAX 10

int input(void);
int range(int i,int *s1,int *s2,int *s3,int *l1,int *l2,int *l3,int f[MAX],int hf[MAX],int lf[MAX]);
void output(int v,int s1,int s2,int s3,int l1,int l2,int l3,int f[MAX],int hf[MAX],int lf[MAX]);

main()
{int f[MAX],lf[MAX],hf[MAX],i=0,n,a=0,b=0,v,s1,s2,s3,l1,l2,l3;
 char choice, choice2;
 clrscr();
  do{
      do{if(i==10)
	  {   printf("\n\n\t sorry,!!! this array is full " );
	      getch();   }
	  else{ n=input();
		f[i]=n;
		 if(f[i] >=50 )
		  { hf[a]=f[i];
		    a++;         }
		   else
		     { lf[b]=f[i];
			b++;   }
		i++;
	       }

	  printf("\n exit ? (y/n) : ");
	  scanf(" %c",&choice );

	  }while(choice !='y' && choice!='Y' );

    printf("\nsegregating/copying these numbers into two other arrays...."); printf("\n press any key to continue");
    getch();

	v=range(i,&s1,&s2,&s3,&l1,&l2,&l3,f,hf,lf);
	output(v,s1,s2,s3,l1,l2,l3,f,hf,lf);

      printf("\n\nexit (y) or repeat (n) ???? (y/n)  :");
      scanf(" %c",&choice2);

    }while (choice2!='y' && choice2!='Y');
  printf("\n\n\n\n\t\t\tterminating program >>>>>>>>>>.......");

getch();
}

int input(void)
{
  int i;
  printf("\n please enter a number   : ");
  scanf("%d",&i);
  return i;
}


int range(int i,int *s1,int *s2,int *s3,int *l1,int *l2,int *l3,int f[MAX],int hf[MAX],int lf[MAX])
{int n,p,l=0,h=0,a,b,c,A,B,C;
 for(p=0;p < i;p++)
  {
    if(f[i] >=50 )
	    { h++; }
      else
	    { l++; }
  }

for(p=1;p <= i;p++)
 {
   if(f[p] < f[p-1] )
      a=f[p];
   if(f[p-1] < f[p])
      a=f[p-1];
   if(f[p] >f[p-1])
      A=f[p];
   if(f[p-1] > f[p] )
      A=f[p-1];
  }


for(p=1;p <= h;p++)
 {
  if(hf[p] < hf[p-1])
      b=hf[p];
  if(hf[p-1] < hf[p] )
      b=hf[p-1];
  if(hf[p] > hf[p-1] )
      B=hf[p];
  if(hf[p-1] > hf[p] )
      B=hf[p-1];
 }

for(p=1;p <=l;p++)
 {
   if(lf[p] < lf[p-1])
      c=lf[p];
   if(lf[p-1] < lf[p])
      c=lf[p-1];
   if(lf[p] > lf[p-1])
      C=lf[p];
   if(lf[p-1] >lf[p] )
      C=lf[p-1];
 }

 *s1=a;
 *s2=b;
 *s3=c;
 *l1=A;
 *l2=B;
 *l3=C;

 n=i;
 return n;
}

void output(int v,int s1,int s2,int s3,int l1,int l2,int l3,int f[MAX],int hf[MAX],int lf[MAX])
{ int i,sum1=0,sum2=0,sum3=0,l=0,h=0;
 float ave1,ave2,ave3;
  for(i=0;i <= v;i++)
   {  if(f[i] >=50)
	 h++;
	    else
		l++;   }
 for(i=0;i <= v; i++ )
   { sum1+=f[i]; }
 for(i=0;i <= h; i++ )
   { sum2+=hf[i];}
 for(i=0;i <= l; i++)
   { sum3+=lf[i]; }

 ave1=sum1/v;
 ave2=sum2/h;
 ave3=sum3/l;


 printf("\n\t the numbers in array1 \n");
   for(i=0; i < v;i++ )
     { printf("\t %d", f[i] );    }
    printf("\n\t\t smallest number : %d",s1);
    printf("\n\t\t largest number : %d",l1);
    printf("\n\t\t sum : %d",sum1);
    printf("\n\t\t average : %.2f",ave1);

  printf("\n\t the numbers in array1 \n");
   for(i=0; i < l;i++ )
     { printf("\t %d",lf[i] );   }
    printf("\n\t\t smallest number :  %d",s2);
    printf("\n\t\t largest number  :  %d",l2);
    printf("\n\t\t sum :   %d",sum2);
    printf("\n\t\t Average :  %.2f", ave2);

  printf("\n\t the numbers in array1 \n");
   for(i=0; i < h;i++ )
     { printf("\t %d", hf[i] ); }
    printf("\n\t\t Smallest number : %d",s3);
    printf("\n\t\t Largest number : %d",l3);
    printf("\n\t\t Sum :  %d",sum3);
    printf("\n\t\t Average :  %.2f",ave3);

getch();
clrscr();
getch();
}

please help.

[griswolf]

OK, I see the red code (by the way, how did you manage to get code without line numbers? The appropriate thing to do is press the (code) button, then paste your code between the tags... or paste first, highlight it all, then press the button)

Based on the red text, I figure that has to be the problem area. But you never said what the problem actually is: Does not "work fine" isn't a very specific issue.

[mtatadotcom]

the red part is suppposed to compute the smallest and largest numbers, but it doesnt, it gives random results

[griswolf]

The usual way of computing min and max goes like this, in pseudocode:

cur_min = a-number-at-least-as-big-as-the-data-minimum // INT_MAX works
cur_max = a-number-at-least-as-small-as-the-data-maximum // INT_MIN works
for every number in the array do this:
  it the_number > cur_max then cur_max = the_number
  if the_number < cur_min then cur_min = the_number
  // You can do other stuff too while looping
end-of-loop

I have looked (briefly) at your code and to be honest, I don't know what it does. Looks like you are trying to re use some kind of sorting algorithm maybe? I think, without spending much time on it, that your code might find the min and max if the data were monotonic, or maybe piecewise monotonic. Maybe?

Let me also point out a few things you might want to think about changing

• explicitly declare main as int main()
• compile with warnings set pretty strong: It is good that the compiler tells you what might not work as you expected.
• declare locale variables one per line (except loop counters, maybe). Comment to explain their use in the scope
• pre-increment by habit: ++i rather than i++ . Post-increment requires a temporary variable to hold the old value. Most compilers have peephole optimizers that make this unimportant for integers and pointers, but you might as well say what you mean; and someday it might matter.
• where you say n = i; return n; why not just return i; instead?
• name your variables something useful. We all use i,j,k for loop counters, but all your other single-letter variables are not at all self documenting. The cost of using good names is very very tiny (you have to type more; and the compiler probably spends another millisecond or so parsing them all). The benefits are real: Self documenting code is easier to write, easier to understand later, easier to debug (and less debugging is needed because with good variable names you sometimes see bugs before they happen) and easier to explain to others (and yourself).

[WaltP]

The usual way of computing min and max goes like this, in pseudocode:

cur_min = a-number-at-least-as-big-as-the-data-minimum // INT_MAX works
cur_max = a-number-at-least-as-small-as-the-data-maximum // INT_MIN works

I would start out slightly differently:

cur_min = first-number-in-the-array 
cur_max = first-number-in-the-array

This way you don't have to guess at a hopefully large/small enough number. And the first number in the array could be the largest or smallest. And it's part of the list anyway.

[griswolf]

I would start out slightly differently:

cur_min = first-number-in-the-array 
cur_max = first-number-in-the-array

This way you don't have to guess at a hopefully large/small enough number. And the first number in the array could be the largest or smallest. And it's part of the list anyway.

Sure, but (and slightly mis-using 'my way' and 'your way' to make the discussion simpler):

• Your way fails for an empty array, mine works and sets the min and max "returns" to obvious sentinel values.
• If you know the type of the array you can guarantee sentinel values that cannot be exceeded. INT_MAX for instance, and similarly for minimums and other types.
• My way makes a redundant check that sets the min and max to the 0th value inside the loop. Your way makes a redundant check that the 0th values are not greater or less than themselves (or it pays a price in extra logic to be sure there's another value to check)

I made a deliberate choice, thinking that my explanation was simpler. I'm not entirely sure it isn't better too, but I doubt it matters much one way or the other.

[mtatadotcom]

thanks, i will work with all your sollutions and see what will bring the desired result in this case
its all about the result not how you got it in programing isnt it ???

thanks anyway

[WaltP]

thanks, i will work with all your sollutions and see what will bring the desired result in this case
its all about the result not how you got it in programing isnt it ???

thanks anyway

Not necessarily. You can get to New York to Philadelphia by way of Cleveland, but just because you got there, is that an acceptable route?

[griswolf]

its all about the result not how you got it in programing isnt it ???

In the real world of professional programming you are both right and wrong. Yes, its really only the results that matter, but "results" aren't just the output of the program...

A huge part of the effort over the life of a piece of software is put into initial programming, but maintenance (and documentation). You will find yourself doing maintenance on code that someone else wrote (maybe yourself, but you don't remember details now) for most of the time you spend programming. So good self-documenting code that is well formatted, appropriately commented and uses best practices costs a little extra in the initial coding phase, but it saves time (and time == money) day after day during the lifetime of the code. Managers care about maintenance costs.

Code that gets the right answer but takes more computer resources costs those extra resources every time it runs. Managers care about resources. Even if your code is only part of what is going on, if it is not efficient it might be the reason that the machine needs to be upgraded for speed or size.

Even efficient and well documented code, if it does something very odd, may be hard to maintain: Doing things the way maintainers expect them to be done is usually better than something idiosyncratic that is otherwise just as good.

If you are wondering "why is maintenance so important?" you should consider these things:

• Sometimes a bug isn't found until (years?) after the code is in production because it is only triggered with recent data.
• Similarly a bug may only be apparent when someone who cares about that particular aspect looks closely.
• Pretty often, a useful body of code needs to be adapted to run on another platform or a newer OS.
• People who use a program may find that it is hard to use the way it is, or needs another feature to be more effective.
• There may have been a plan to roll out features in phases because of a deadline for the first features, or because of cost or other constraints.
• A useful program may need to be adapted to new business situations.

(This has certainly gotten off topic, hasn't it?)

[mtatadotcom]

hello friends, thank u all for your advice, i would like to share with u the protion of the code that gave me trouble, i works now perfectly and a few minor adjustments i had tto ma

1. i added a new subprogram incase the any of the two arrays was empty

void nullarray(int v,int f[MAX]);
.
.
.
if(a!=0 && b!=0)           /*displays only if all arrays have alteast one value*/
	  {  clrscr();
	    range(i,a,b,&s1,&s2,&s3,&l1,&l2,&l3,f,lf,hf);
	    output(i,s1,s2,s3,l1,l2,l3,f,hf,lf);                 }
	else
	 { nullarray(i,f); }     /*incase of null input of any array after segregation */
.

and this is how i aplied the sorting algorithm for smallest and largest < by passing value by reference >

void range(int v,int a, int b,int *s1,int *s2,int *s3,int *l1,int *l2,int *l3,int     f[MAX], int lf[MAX], int hf[MAX])
{int i,small1=f[v-1],small2=lf[b-1],small3=hf[a-1],large1=small1,large2=small2, large3=small3;
 for(i=0;i < v;i++ ){ if( f[i] < small1)
			small1=f[i];
		       if( f[i] > large1)
			 large1=f[i];      }

 for(i=0;i < b;i++ ){ if( lf[i] < small2) 
			small2=lf[i];
		       if( lf[i] > large2)
			 large2=lf[i];      }

 for(i=0;i < a;i++ ){ if( hf[i] < small3) 
			 small3=hf[i];
		       if( hf[i] > large3)
			 large3=hf[i];      }

*s1=small1;
*s2=small2;
*s3=small3;
*l1=large1;
*l2=large2;
*l3=large3;
}

i passed all the smalls and large to the output funtion

thanks for all your help, although it seemed to get a out of topic