using function to find max value in an array

Question

Can someone please explain what I'm doing wrong. I need to use an array to find the max value of the percentages array and display that amount and the year in the corresponding years array.

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

int findHighNumber (double percentages[], int elements);

int main (void)
{

  int index;
  int elements;


  int years[] = {2000, 2002, 2004, 2006, 2008, 2010, 2012};
  double percentages[] = {6.7, 6.6, 8, 9, 11.3, 14.7, 14.6};
  int sampleSizes[] = {187761, 444050, 172335, 308038, 337093, 1000, 346978};


   int  high = findHighNumber(percentages, elements);
   printf ("%i had the highest percentage with autism at %.2lf%%.\n", years[high], percentages[high]);


   return 0;
}

 int findHighNumber (double percentages[], int elements)
 {
    int index, high;
    high = percentages[0];

    for (index = 1; index < elements; index++)
       if (high < percentages[index])
       {
        high = percentages[index];
    }
   return high;
 }

You need to store the index of the max value, along the value itself, and return it. I would also rename the function to findIndexOfMax to make it obvious what it should be doing. — Lou
– Lou, Commented Mar 15, 2017 at 8:06

ncarrier · Accepted Answer · 2017-03-15 08:07:27Z

First, you have to declare the elements variable in main():

int elements = 7;

or even better:

int elements = sizeof(percentages) / sizeof(*percentages);

which computes the array size automatically.

Then, don't modify the elements variable in you loop. If you do it, each time the current maximum is changed, you program your loop to stop after the next element, potentially missing the real maximum which can be located at the end of the array.

Then, like Bob said, you should return the index of the maximum, this way, you can retrieve the corresponding sampleSize, or year.
When you do years[high], you're using a percentage as an array index which can take any real value in [0,100], instead of an index which must stay in {0, 1, 2, 3, 4, 5, 6}.

What's more, you store high in an integer variable, resulting in a truncation of it's value. You may prefer to store it as a double variable.

So findHighNumber() can become:

    int findHighNumber(double percentages[], int elements)
    {
            int index;
            double high;
            int high_index;

            high_index = 0;
            high = percentages[high_index];

            for (index = 1; index < elements; index++)
                    if (high < percentages[index]) {
                            high = percentages[index];
                            high_index = index;
                    }

            return high_index;
    }

The main() function now can become something like:

    int main (void)
    {
            int years[] = {2000, 2002, 2004, 2006, 2008, 2010, 2012};
            double percentages[] = {6.7, 6.6, 8, 9, 11.3, 14.7, 14.6};
            int sampleSizes[] = {187761, 444050, 172335, 308038, 337093, 1000,
                    46978};
            int elements = sizeof(percentages) / sizeof(*percentages);
            int high_index = findHighNumber(percentages, elements);

            printf ("%i had the highest percentage with autism at %.2lf%% (sample"
                    "size was %d).\n", years[high_index], percentages[high_index],
                    sampleSizes[high_index]);

            return 0;
    }

Then, some small advices:

putting a prototype of findHighNumber() is not needed, just define the function above the places where it's needed. This way, when you really have to put a prototype for a function, it a hint saying that you were forced to do so, (for example for mutually recursive functions), plus it shortens the code.
you should define a, for example, struct autism_sample like that:
```
struct autism_sample {
        int year;
        int sample_size;
        double percentage;
};
```
This way, you just have to define one array:
```
struct autism_sample autism_samples[] = {
    {
        .year = 2000,
        .sample_size = 187761,
        .percentage = 6.7,
    },
    {
        .year = 2002,
        .sample_size = 444050,
        .percentage = 6.6,
    },
    ...
};
```
This way, your data is organized in a more logical way, less error-prone to maintain and you gain the choice, in the implementation of findHighNumber(), to return either the index of the maximum or directly, a pointer to the element holding the maximum, the index becomes then useless.
What's more, it's easier to (un)serialize...

Natasha Dutta · Accepted Answer · 2017-03-15 07:04:38Z

2

I'm not sure if this is a typo here, but it appears while calling

 int  high = findHighNumber(percentages, elements);

you don't have a variable elements defined there. That said, you have another syntax error (missing the closing }) near

if (high < percentages[index])
       {
        high = percentages[index];  ///after this.

Finally, inside findHighNumber (), elements is local to the function, so, the only usage as elements = index+1; is useless.

answered Mar 15, 2017 at 7:04

Natasha Dutta

3,27223 silver badges23 bronze badges

1 Comment

Jessica Blake Over a year ago

Thanks! I've fixed what you've mentioned and I'm still not getting the correct numbers.

Bob__ · Accepted Answer · 2017-03-15 07:11:51Z

1

Given the way you are storing and using data, you should return the index where the higher element is, not its value. Also you have to pass the right size of the array (and don't change it inside the function), while you are passing an uninitialized value.

answered Mar 15, 2017 at 7:11

Bob__

12.9k3 gold badges30 silver badges44 bronze badges

2 Comments

Jessica Blake Over a year ago

I'm about 4 weeks into programming so bear with me but I'm not exactly sure what mean by return the index where the higher element is. Cold you show an example? I removed the "elements = index +1"

Bob__ Over a year ago

@JessicaBlake It looks like ncarrier already resolved your doubts ;)

jyap · Accepted Answer · 2017-03-15 08:07:06Z

1

You should send the index instead of the value.

int findIndexOfMax (double percentages[], int elements) {

    int index = 0;
    int highIndex = 0;
    double high = 0.0;
    high = percentages[highIndex];

    for (index = 1; index < elements; index++) 
    {
        if (high < percentages[index]) 
        {
            high = percentages[index];
            highIndex = index;
        }       
    }

    return highIndex;
}

edited Mar 15, 2017 at 8:07

answered Mar 15, 2017 at 8:01

jyap

1,6041 gold badge12 silver badges5 bronze badges

Comments

Wanderer · Accepted Answer · 2017-03-15 08:08:11Z

0

You are returning the wrong value .. you should not return what's the highest value but you should return the index of it.

or you you can simply print the result that you want in the function you're calling.

Also, I don't see that you have initialized the variable elements.

answered Mar 15, 2017 at 8:08

Wanderer

1,2076 gold badges21 silver badges44 bronze badges

Collectives™ on Stack Overflow

using function to find max value in an array

5 Answers 5

1 Comment

1 Comment

2 Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

1 Comment

1 Comment

2 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest