3 Highest Numbers in an array without sorting, Where am I missing the logic?

Question

This program is for finding 3 highest numbers in an array.

When I run the code, I'm getting first highest and second highest. And second highest is getting repeated for third number

What am I missing in the logic?

#include<stdio.h>
#include<conio.h>

int main()
{
   int i,k,n,m[20],h[3];
   printf("\n enter the total number of students");
   scanf("%d",&n);
   for(i=0;i<n;i++)
   {
      printf("enter the marks scored by student %d",i+1);
      scanf("%d",&m[i]);
   }//end for loop
   k=0;
   h[k]=m[0];
   for(i=0;i<n;i++)
   {
      if(m[i]>h[k])
      {
         h[k]=m[i];
      }
   }//end for loop
   do
   {
      //Probably messed my code here
      k++;
      h[k]=m[0];
      for(i=0;i<n;i++)
      {
         if(m[i]>=h[k-1])
         {
            if(m[i]>h[k])
            {
               h[k]=m[i];
            }//end if
            break;
         }//end if
      }//end for loop
   }//end do loop
   while(k<3);
   printf("the first 3 highest marks are:\n");
   for(i=0;i<k;i++)
      printf("%d:%d\n",i+1,h[i]);
   getch();
}//end of main

There are two ways to solve problems like this: 1) Run your code in a debugger and step through it one line at a time, watching variable values, until you find out where it's not doing what you want; 2) Insert output statements (`printf`) at strategic points in the code to see what's happening. The way SO works is NOT: "Here's my code, please debug it for me". You debug it and then ask if you still don't understand, after explaining what you've done. — Jim Garrison, Sep 28 '15 at 05:10
Is it just top 3 numbers you want or it can be more ( in future) ? — Nitin Tripathi, Sep 28 '15 at 05:11
and is list always going to be a collection of unique numbers? — Nitin Tripathi, Sep 28 '15 at 05:13
Bonus question: Why is one member of m[] changed in the last step of the do-while loop? — Sleafar, Sep 28 '15 at 05:19

Nitin Tripathi · Answer 1 · 2015-09-28T05:58:52.597

3

I don't know, what is your purpose behind writing such a hugh lines of code if this can be done in much simpler way. Here is another approach of finding Top 3 numbers in just one iteration; Hope you may like it and it might help you in future.

SOURCE:

#include<stdio.h>
#include<limits.h>
int main()
{
    int a[10] = {5,4,3,6,7,8,9,10,1,2};
    int h1 = INT_MIN; //TOP 1st
    int h2 = INT_MIN; //TOP 2nd
    int h3 = INT_MIN; //TOP 3rd
    for(int i=0;i<10;i++)
    {
        if(a[i] > h1)
        {
            h3 = h2; h2 = h1; h1 = a[i];
        }
        else if (a[i] > h2)
        {
            h3 = h2; h2 = a[i];
        }
        else if  (a[i] > h3)
        {
            h3 = a[i];
        }
    }
    printf("TOP 1st is<%d>\n",h1);
    printf("TOP 2nd is<%d>\n",h2);
    printf("TOP 3rd is<%d>\n",h3);
    return 0;
}

OUTPUT:

./a.out 
TOP 1st is<10>
TOP 2nd is<9>
TOP 3rd is<8>

edited Sep 28 '15 at 05:58

answered Sep 28 '15 at 05:23

Nitin Tripathi

1,224
7
17

2

`int a[10] = {5,4,3,9,6,8,2,7,1};` – user3386109 Sep 28 '15 at 05:27
correction, adding one more element in array to make it list of 10 elements :) – Nitin Tripathi Sep 28 '15 at 05:29
2

@NitinTripathi, that's not the problem. The problem is that your logic doesn't work if the largest number appears before the second and third largest numbers. – R Sahu Sep 28 '15 at 05:31
I will take my solution back. Thanks for pointing it out. I never thought of it. It will surely require two iterations.. – Nitin Tripathi Sep 28 '15 at 05:33
1

You man make a minor change to make it work. – R Sahu Sep 28 '15 at 05:34
1

`int a[10] = {10,4,3,6,7,8,9,5,1,2};` – user3386109 Sep 28 '15 at 05:40
1

@user3386109, Thanks for pointing out the issue, Correcting the initialisation with INT_MIN defined in limits.c. hope it will work now. Thanks again. – Nitin Tripathi Sep 28 '15 at 05:59
1

@NitinTripathi Thank a lot. It is so simple. – Pravin Hanchinal Sep 30 '15 at 10:47

score 0 · Answer 2 · answered Sep 28 '15 at 05:24

I see the following problems:

The line
```
h[k]=m[0];
```
will lead to problems if m[0] is the largest or the second largest value. Change it to:
```
h[k]=INT_MIN;
```
The line
```
 if(m[i]>=h[k-1])
```
seems wrong. That should be
```
 if(m[i] < h[k-1])
```
The line
```
    break;
```
is a source of error. It will fail to detect the second and third largest values correctly if they are towards the end of the list.

Here's the do/while block with those fixes.

do
{
   k++;
   h[k]=INT_MIN;
   for(i=0;i<n;i++)
   {
      if(m[i] < h[k-1])
      {
         if(m[i]>h[k])
         {
            h[k]=m[i];
         }//end if
      }//end if
   }//end for loop
}//end do loop

It seems to work for me.

Al Pacifico · Answer 3 · 2015-09-28T05:36:31.687

One of your problems is with the line

if(m[i]>=h[k-1])

This condition is going to be met once per iteration of your do...while loop (assuming the highest mark occurs just once), and the next line is going to be executed only if the highest number is not the first one.

It's not the only problem with the code. There are lots of conditions under which this code isn't going to work right, alluded to by Nitin Tripathi's comments.

I suggest you load the h array with the first three marks. Then, sort it using a bubble-sort algorithm so that h[0] is the highest and h[2] is the lowest. Then iterate over the remaining values of the marks. For each value, if it is higher than h[2], replace h[2] with it and re-sort the h array using the same bubble-sort algorithm. You can just google that and find lots of examples.

Commenting your code would probably help you a lot, as well.

score 0 · Answer 4 · answered Sep 28 '15 at 05:41

You need to sort h array, following code working fine

#include<stdio.h>
#include<conio.h>

void main()
{
   int i,a,j,k,n,m[20],h[3];
   int high, temp;
   printf("\n enter the total number of students");
   scanf("%d",&n);
   for(i=0;i<n;i++)
   {
      printf("enter the marks scored by student %d",i+1);
      scanf("%d",&m[i]);
   }//end for loop

   k=0;
   h[k]=m[0];

   for(i = 1; i < n; i++)
   {
      if(m[i] > h[0])
      {
     h[k] = m[i];
     k++;

     // make sure h[0] have max value out of all 3
     for (a = 0; a < k; a++)
     {
        for (j = a + 1; j < k; j++)
        {
            if (h[a] < h[j])
            {
                temp = h[j];
                h[j] = h[a];
                h[a] = temp;
            }
        }
     }
      }
   }//end for loop


   for (i = 0; i < k; i++)
   {
    printf("\n %d", h[i]);
   }

   getch();
}

score 0 · Answer 5 · answered Sep 28 '15 at 06:22

0

#include <algorithm>
#include <cstddef>

template<typename RanIt>
void make_N_highest( std::size_t N, RanIt b, RanIt e )
{
    std::make_heap(b,e);
    while(N--)
        std::pop_heap(b,e--);
}

This will put the N highest values last in the sequence.
It's O( N * log(size) )

answered Sep 28 '15 at 06:22

sp2danny

7,488
3
31
53

[live demo](http://coliru.stacked-crooked.com/a/ef093881a09722fa) – sp2danny Sep 28 '15 at 06:27

3 Highest Numbers in an array without sorting, Where am I missing the logic?

5 Answers5