4
\$\begingroup\$

My goal was to create a function that could check for duplicates in an array no matter what the data type was, given that the size of the data type was provided. This was done by using the \$\mathcal{O}(n^2)\$ algorithm and casting to char because of its 1 byte property.

/**
 * Returns count of duplicates in an array
 * @param  a     array to check in
 * @param  items how many entries in array
 * @param  size  size of each entry
 * @return       returns amount of duplicates/conflicts
 */
int dubCount(void *a, size_t items, size_t size){
    int i,j;
    int conflicts = 0;
    char *x = (char *)a;

    for (i = 0; i < items - 1; i++){
        for (j = i + 1; j < items; j++){
            /* Check if chunks are equal, if so count */
            if(memcmp(&(x[i*size]), &(x[j*size]), size) == 0){
                conflicts++;
            }
        }
    }

    return conflicts;
}

The code has been confirmed to work with many data types. As I'm quite new to the C world, I'd like for someone to point out anything wrong or dangerous in this function.

\$\endgroup\$
2
  • \$\begingroup\$ It won't work correctly for floating point types. It will show two NaNs as being equal (even though they should compare as not equal). It can also show two variable both equal to zero as being unequal (multiple bit patterns can give the value 0). \$\endgroup\$ Dec 12, 2016 at 4:58
  • \$\begingroup\$ You forget about padding in objects. Doing a memcmp() on two identical objects may return false of the padding bits are not explicitly zeroed out (not sure the standard guarantees this). This is why qsort takes a third parameter (a function) that allows you to compare objects. \$\endgroup\$ Dec 12, 2016 at 5:07

1 Answer 1

2
\$\begingroup\$

This is a very straightforward implementation which is great for maintainability and readability. Here are some things you could improve:

Not All Comparisons Work In Obvious Ways

As mentioned in the comments by Jerry Coffin and Loki Astari, you may run into issues with floating point values or values which may not be padded how you think, or for which the padding contains random values. You could solve this by also passing in a pointer to a comparison function and calling that function to do the comparison.

Naming

Your naming could be improved a little. The function is named dubCount(), but from the comments and the name of the return value it should probably be conflictCount() or at least dupeCount(). It's not clear what dub refers to in dubCount() (at least to me). Also the parameter name a tells me nothing, and since it's a pointer to void, I have no contextual clue, either. I'd rename it to something like items and rename the items parameter to numberOfItems or numItems, and rename size to itemSize or sizePerItem or bytesPerItem so that it's more clear just from the function prototype what the parameters mean.

Use const where appropriate

In this function, none of the parameters to the function are modified. When your function doesn't modify a parameter, it should be marked as const. This lets both the compiler and a reader know the function won't change those values. The compiler may be able to make some optimizations with that info, and a reader can do more reasoning about the function without reading its actual code, which is helpful.

Performance

You noted that the algorithm you're using is \$\mathcal{O}(n^2)\$. If you sorted the items in the array first, you could use an O(n) search on the result as you'd be able to simply run through the array until you hit a different value. (Note that this may depend on the statistical nature of your input data and the sort algorithm you use. But in general, I'd expect to see a speedup in such a case.) Most fast sort algorithms are \$\mathcal{O}(n * log(n))\$ so the result would be \$\mathcal{O}(n + n * log(n))\$ (I think? Check my math). Note that you would use the same comparison function for the sort as you would for checking for duplicates.

\$\endgroup\$
1
  • \$\begingroup\$ Thank you for your feedback. I agree on all points and all your points will surely be implemented. Also thanks to Jerry Coffin and Loki Astari for pointing out the problem floating point values. \$\endgroup\$
    – Napoleon
    Dec 12, 2016 at 9:10

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.