HiveBrain v1.2.0
Get Started
← Back to all entries
patterncMinor

Counting occurrences of duplicate items in an array

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
countingoccurrencesarrayduplicateitems

Problem

I have to make a simple program that reads N integers from the user and counts the occurrences of duplicate items. I know that the chosen sorting algorithm has poor performance, but I chose it for its simplicity.

I'm looking for suggestions for the last part, in which I count the duplicates.

#include 

#define N 10

int main(void) {
    int i, values[N];
    puts("Insert the values:");
    for (i = 0; i  0) dupn++;
    }
    for (i = 0; i < dupn; i++) {
        printf("Duplicate %d occurs %d times\n", duplicates[i], dupocc[i]);
    }
    return 0;
}

Solution

There are a few things that stand out to me immediately in your code. Some good, some are troubling.
Style

First up, your code is neat, and well structured. Indentation is good, and I even appreciate the fact that you have braced 1-liners and got the space right in your expressions.

Traditionally, C puts the open brace on the new line.... I am not a fan of that, but I don't write C very often, so I stick with a style I use in Java. Just thought I would point that out, though.
Sorting

Your sort algorithm is ... basic, but effective. I would agree that it is a problem. As a suggestion, you should use an insertion sort, and do the sort at the same time as the scanf. That way you load the data in an always-sorted way:

puts("Insert the values:");
for (i = 0; i  0 && values[pos - 1] > val) {
        values[pos] = values[pos - 1];
        pos--;
    }
    values[pos] = val;
}


The above will do a shifting insertion sort. Just a suggestion.
Duplicates.

This is where there are the most significant problems. I would consider your algorithm to be a real overkill one. You can do it all with no extra data, no duplicates[] array, etc. A simple loop through the data should be fine.

int count = 1;
int previous = values[0] + 1; // something different to the start.
for (int i = 0; i  1) {
            // report the count of previous duplicates.
            printf("Duplicate %d occurs %d times\n", previous, count);
        }
        previous = values[i];
        count = 0;
    }
    count++;
}
if (count > 1) {
    // catch the last count, if any
    printf("Duplicate %d occurs %d times\n", previous, count);
}

Code Snippets

puts("Insert the values:");
for (i = 0; i < N; i++) {
    int val;
    scanf("%d", &val);
    int pos = i;
    while (pos > 0 && values[pos - 1] > val) {
        values[pos] = values[pos - 1];
        pos--;
    }
    values[pos] = val;
}
int count = 1;
int previous = values[0] + 1; // something different to the start.
for (int i = 0; i < N; i++) {
    if (values[i] != previous) {
        // something changed... reset the counters
        if (count > 1) {
            // report the count of previous duplicates.
            printf("Duplicate %d occurs %d times\n", previous, count);
        }
        previous = values[i];
        count = 0;
    }
    count++;
}
if (count > 1) {
    // catch the last count, if any
    printf("Duplicate %d occurs %d times\n", previous, count);
}

Context

StackExchange Code Review Q#88546, answer score: 2

Revisions (0)

No revisions yet.