0

For school I have to create a function that prints the mode of a sorted array. It has to account for there being multiple modes and there being no modes as well. For some reason even if there is a mode in the array the function always prints "no mode"

void findMode(double *mode, double a[], unsigned int size)
{
    double number = a[0]; //Used to to compare values in the array to see if they're similar
    int count = 1; //Keeps track of number of occurences for each number
    int max = 1; //Keeps track of max number of occurences found for each number
    int uniqueNum = 1; //Keeps track of how many unique values in the array
    int maxCount = 1; //Counts how many set's of numbers occur the max ammount of times
    int elementNum = 0; //Keeps track of element number in mode array

    for (unsigned i = 1; i < size; ++i)//loop to determine how many modes and unique numbers there are
    {
        if (number == a[i])
        {
            ++count; //if the numbers the two numbers compared are the same, count is increased by 1
        }
        else
        {
            if (count == max)
            {
                ++maxCount; //Keeps track of how many modes are in the array
            }
            if (count > max)
            {
                //If the two numbers compared are not the same and the count of the previous "set" of similar numbers is higher than the current max count, max is equal to the new count
                max = count;
                maxCount = 1; //Reset the max count if a new max is found
            }
            //Count is set back to 1 as we are counting a different number
            count = 1;
            number = a[i];
            ++uniqueNum; //Unique number variable gets incremented
        }
    }

    printf("\n%d, %d, %d", max, maxCount, uniqueNum);
    count = 1; //sets count back to 1 for next loop

    if ((double)size / max == uniqueNum)
    {
        mode = malloc(1);
        mode[0] = (a[size - 1] + 1); //Returns a value that can't exist in the array there is no mode
    }
    else
    {
        mode = malloc(sizeof(double) * maxCount); //makes the mode array the right size to store all the modes
        for (unsigned i = 1; i < size; ++i)//loop to determine what the modes are
        {
            if (number == a[i])
            {
                ++count; //if the numbers the two numbers compared are the same, count is increased by 1
            }
            else
            {
                if (count == max)
                {
                    mode[elementNum] = a[i];
                    ++elementNum;
                }
                //Count is set back to 1 as we are counting a different number
                count = 1;
            }
        }

        if (mode[0] = (a[size - 1] + 1))
        {
            printf("\nMode: no mode");
        }
        else
        {
            printf("\nMode: {");
            for (int i = 0; i <= (sizeof(mode) / sizeof(mode[0])); ++i)
            {
                printf(" %.3lf ", mode[i]);
            }
            printf("}");
        }
    }

}

I have no clue how close or far away I am from the correct logic in this, so if I'm way way off and need to start from scratch let me know.

Thanks!

Frankjoww
  • 45
  • 5
  • I think you need to start from scratch with learning how to use a debugger. – Martin James Feb 19 '16 at 19:37
  • 1
    The array-indexing loop starting at 1 does not fill me with confidence:( – Martin James Feb 19 '16 at 19:38
  • @MartinJames I saw that but he reads index 0 first. But what is a **mode**? – Weather Vane Feb 19 '16 at 19:39
  • Start from scratch again. My advice is that instead of testing full code next tiime when you write the code break it in micro parts taking into account of dependencies and then keep on testing them. It will be easy for you – Kevin Pandya Feb 19 '16 at 19:40
  • @MartinJames it's because a already have the first element of the array stored in another array for comparing purposes, I figured there was no point in starting at 0 and comparing the the first element in the array to itself. – Frankjoww Feb 19 '16 at 19:40
  • @KevinPandya Alright, doesn't sound like the worst idea. I'm starting to think finding the maxCount and uniqueNum should be in their own functions (if I even need them) – Frankjoww Feb 19 '16 at 19:42
  • Why are you assigning the result of `malloc` to the function argument `*mode`? This is the first use of this arg, it will not find its way back to the caller anyway. If that is the value of interest to the phantom caller, return that as the function value instead of `void`. – Weather Vane Feb 19 '16 at 19:42
  • You mean the `mode = malloc(1);`? Sloppy coding I guess, I'll change it. – Frankjoww Feb 19 '16 at 19:46
  • There is no `free` to balance `malloc` so presumably you intend the caller to get `*mode`, but it won't. The function was passed a *copy* of a pointer, of which you make no use apart from treating as a local variable. – Weather Vane Feb 19 '16 at 19:47
  • Try to make one note. Always keep main function as compact as possible. Make functions for all the task and in main function only include them. So you can first try to break your current code in functions and see if the code runs. But don't waste more time on same if nothing works out – Kevin Pandya Feb 19 '16 at 19:49
  • No mode exists if no number is repeated. I'm not sure what your line `if (mode[0] = (a[size - 1] + 1))` is trying to do but whether or not a mode exists should never depend on a value within the array. – mban Feb 19 '16 at 19:58
  • @WeatherVane so should I maybe be declaring my mode array inside the function instead of passing it in? – Frankjoww Feb 19 '16 at 20:00
  • @mban Alright, I'll change it to work with a boolean instead – Frankjoww Feb 19 '16 at 20:02
  • @Frankjoww you overwrote the passed arg `*mode` with the `malloc`, so as I said, you treat it as a local variable, which does not find its way back. Since you did not provide a [MCVE](stackoverflow.com/help/mcve) to show how the function is called, you may or may not be able to write to `*mode` directly. – Weather Vane Feb 19 '16 at 20:07

1 Answers1

2

Your problem is your comparison if (mode[0] = (a[size - 1] + 1))

The single = is doing assignment not equivalence testing. You assign mode[0] to the value of a[size -1] + 1

The assignment operator in c returns the value of the left operand as you can read in the answer to this question. In your case it will return a positive number which will be cast to a boolean and be evaluated as true every time.

Community
  • 1
  • 1
Rainbacon
  • 933
  • 1
  • 16
  • 24
  • Yeah, i realized that it had something to do with that, and changed it to work with a boolean because it's just easier (and I'm assuming more proper) that way. Now I do get a number output when there is a mode, however it's some weird { -674285437328543.124543543} or something like that. I'm thinking it's a pointer/memory allocation problem. I'll keep playing around with it and see if I can fix it – Frankjoww Feb 19 '16 at 20:07