1

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;
 }
2
  • 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. Commented Mar 15, 2017 at 8:06
  • Thank you guys! I got it to work. Commented Mar 15, 2017 at 8:18

5 Answers 5

2

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:

  1. 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.
  2. 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...

Sign up to request clarification or add additional context in comments.

1 Comment

Thank you so much!
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.

1 Comment

Thanks! I've fixed what you've mentioned and I'm still not getting the correct numbers.
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.

2 Comments

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"
@JessicaBlake It looks like ncarrier already resolved your doubts ;)
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;
}

Comments

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.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.