Polling an Array

I want to find the value occurs most in an 8 integer array. Here is the code that I have. It does compile, but has not been tested.

Code:

int POLL (int *RANGE_ARRAY) { int d = 0; int e = 0; int f = 0; int g = 0; int h = 0; int j = 0; int k; int m = 0; int n = 0; int p; for (k = 0; k < 8; k++) { if (RANGE_ARRAY[0] == RANGE_ARRAY[k]) d++; if (RANGE_ARRAY[1] == RANGE_ARRAY[k]) e++; if (RANGE_ARRAY[2] == RANGE_ARRAY[k]) f++; if (RANGE_ARRAY[3] == RANGE_ARRAY[k]) g++; if (RANGE_ARRAY[4] == RANGE_ARRAY[k]) h++; if (RANGE_ARRAY[5] == RANGE_ARRAY[k]) j++; if (RANGE_ARRAY[6] == RANGE_ARRAY[k]) m++; if (RANGE_ARRAY[7] == RANGE_ARRAY[k]) n++; } if (d >= (e & f & g & h & j & m & n)) p = 0; if (e >= (d & f & g & h & j & m & n)) p = 1; if (f >= (e & d & g & h & j & m & n)) p = 2; if (g >= (e & f & d & h & j & m & n)) p = 3; if (h >= (e & f & g & d & j & m & n)) p = 4; if (j >= (e & f & g & h & d & m & n)) p = 5; if (m >= (e & f & g & h & j & d & n)) p = 6; if (n >= (e & f & g & h & j & m & d)) p = 7; return (p); }

Does this look right or is there a better way of doing this?

I haven't really looked at your code, but few questions.

Why haven't you tested if it is correct?

Also, you haven't stated in which intervals your integers can be, but I assume [0, 7]?

If your assignment was to do this for a array of 100 integers, would you still do it the same way? Hint, I hope you wouldn't. Programming is about making the computer do all the work. It would be simpler to do it with pen and paper instead of scaling your method.

I suggest using an array instead of several variables.

Quote:

Originally Posted by Jimmy

I haven't really looked at your code, but few questions.

Why haven't you tested if it is correct?

Also, you haven't stated in which intervals your integers can be, but I assume [0, 7]?

If your assignment was to do this for a array of 100 integers, would you still do it the same way? Hint, I hope you wouldn't. Programming is about making the computer do all the work. It would be simpler to do it with pen and paper instead of scaling your method.

I suggest using an array instead of several variables.

I didn't say it is correct, I said it compiles. I have to download it into my equipment, which is out in the cold, to test it.

The integers are [0, 7].

This is the way I came up with to do the job. It's the first cut, and I can refine it as needed. You are right about using an array for the variables. I will make that change. Thanks for the help, that is the kind of help that I'm looking for.

I found one mistake and fixed it.

Code:

int POLL (int *RANGE_ARRAY) { int d = 0; int e = 0; int f = 0; int g = 0; int h = 0; int j = 0; int k; int m = 0; int n = 0; int p; for (k = 0; k < 8; k++) { if (RANGE_ARRAY[0] == RANGE_ARRAY[k]) d++; if (RANGE_ARRAY[1] == RANGE_ARRAY[k]) e++; if (RANGE_ARRAY[2] == RANGE_ARRAY[k]) f++; if (RANGE_ARRAY[3] == RANGE_ARRAY[k]) g++; if (RANGE_ARRAY[4] == RANGE_ARRAY[k]) h++; if (RANGE_ARRAY[5] == RANGE_ARRAY[k]) j++; if (RANGE_ARRAY[6] == RANGE_ARRAY[k]) m++; if (RANGE_ARRAY[7] == RANGE_ARRAY[k]) n++; } if (d >= (e && f && g && h && j && m && n)) p = 0; if (e >= (d && f && g && h && j && m && n)) p = 1; if (f >= (e && d && g && h && j && m && n)) p = 2; if (g >= (e && f && d && h && j && m && n)) p = 3; if (h >= (e && f && g && d && j && m && n)) p = 4; if (j >= (e && f && g && h && d && m && n)) p = 5; if (m >= (e && f && g && h && j && d && n)) p = 6; if (n >= (e && f && g && h && j && m && d)) p = 7; return (p); }

Your second if condition doesn't work correctly. The series of && only return a boolean result, not the value of all of the largest value amongst them. So really all your checking is, is each letter value greater than 1 or 0 which is not what your intention is I believe?

Here is a simple example of how to properly check this:

Code:

int POLL (int * range_array, size_t size) /* take the size of the array, don't use a hard-coded value */ { int frequency[size]; /* Replaces all your individual variables */ size_t index, subcheck; /* Loop control */ memset(frequency, 0, sizeof frequency); /* EDIT: forgot you can't initialize VLAs */ /* For every value in range_array, check it against every other value of range_array and count how many times it appears (should always be at least once) */ for (index = 0; index < size; ++index) for (subcheck = 0; subcheck < size; ++subcheck) if (range_array[subcheck] == range_array[index]) frequency[index]++; /* Now we know the frequency of each item in the array. Lets find the item with the largest frequency now */ int max = 0; int max_index = 0; for (index = 0; index < size; ++index) { if (frequency[index] > max) { max = frequency[index]; max_index = index; } } /* return the item with the largest frequency */ return range_array[max_index]; }

Quote:

Originally Posted by nonpuz

Your second if condition doesn't work correctly. The series of && only return a boolean result, not the value of all of the largest value amongst them. So really all your checking is, is each letter value greater than 1 or 0 which is not what your intention is I believe?

Here is a simple example of how to properly check this:

<snip>

You really shouldn't give out complete solutions like that.

I felt it was warranted in this situation because the OP shows a fundamental misunderstanding of how to accomplish the requested task and it has been over a day since the OP. But you're probably right, should've just left it at the description of the issue.

nonpuz,

Thanks for your help. I have already made a change to the second if condition. I changed 'return (p);' to 'return (RANGE_ARRAY[p]);'. I have tested it in a static condition and it returns a '1', and that's correct, because the array is only loaded with 1s. I have to test it in the equipment to get dynamic data in the array.

Your code is certainly more elegant, and I will use it.

Quote:

Originally Posted by nonpuz

I felt it was warranted in this situation because the OP shows a fundamental misunderstanding of how to accomplish the requested task and it has been over a day since the OP. But you're probably right, should've just left it at the description of the issue.

Well, he will probably use that code without understanding it. I'm not sure if it is for a assignment or not either.

The biggest problem with giving people a solution is that it make them unable to think for themselves. The people that are given a solution like this tend to come back and expect to get a complete solution the next time over and over again instead of learning something and thinking for themselves.

This is NOT an assignment. This is for a real piece of hardware. I understand the code that nonpuz posted, and I use similar code in other sections of this program.

Quote:

Originally Posted by danlee58

This is NOT an assignment. This is for a real piece of hardware. I understand the code that nonpuz posted, and I use similar code in other sections of this program.

Let me guess, you where sitting there, writing code, when you suddenly suffered and aneurysm. So what is a programmer with an aneurysm going to do? "Of course, I post the brain dead code to cboard, at least I used to be a decent programmer".

Lets just say that I don't believe you. It might not be an assignment, but you don't understand that piece of code, you have never written something similar either. Because if you had, then you would not have posted the original post in the first place. Unless you had an aneurysm or something similar.

My compiler doesn't recognize size_t as a variable type, and it gives me this warning:

warning: implicit declaration of function 'memset' [-Wimplicit-function-declaration]
warning: incompatible implicit declaration of built-in function 'memset' [enabled by default]

Otherwise the code posted by nonpuz works. I used 'int size' instead of 'size_t size'.

You need to include the correct headers. For example, size_t is declared in <stddef.h> and some other standard headers like <stdlib.h>, <stdio.h> and <string.h>. memset is declared in <string.h>. So, if you #include <string.h>, those warnings should vanish.