Comparing arrays in a function

Question

My function countCopies doesn't work even though it gets the right inputs. All it should do is taking an array of integers as an input and then searching through this array for duplicates of the second input x.

int main() {
    char intArray[100]; //The integer array can only hold 100 integers
    int i, x, j;
    printf("Please enter a couple of integers and when you're done enter end. ");

    i = 0;
    while (scanf("%d", &intArray[i++]) == 1)
        /*empty loop*/;

    scanf("%*s");

    printf("Enter x:");
    scanf("%d", &x);

    printf("Copies = %d\n", countCopies(intArray, i, x));
}

int countCopies(int a[], int n, int x) {
    int count = 0;
    int j = 0;
    for (j = 0; j < n - 1; j++) {
        if (a[j] == x) {
            count++;
        }
    }
    return count;
}

@ameyCU, see [what is scanf(“%*s”) and scanf(“%*d”) format identifiers?](http://stackoverflow.com/questions/2155518/what-is-scanfs-and-scanfd-format-identifiers) — Box Box Box Box, Feb 27 '16 at 12:12
@AshishAhuja I know what that means but I am confused why is it used here (_I don't see any reason to use it here ._) — ameyCU, Feb 27 '16 at 12:13
@ameyCU: see my answer: `scanf("%*s");` skips the `end` word. As always, non obvious code should be commented. — chqrlie, Feb 27 '16 at 12:17

chqrlie · Accepted Answer · 2016-02-27T12:27:49.723

The for loop is incorrect: you should change the test to j < n. The idiomatic for loop in C: for (j = 0; j < n; j++) ... iterates exactly n times, j taking values 0 to n-1 inclusively, which are correspond to exactly all valid positions in an array of n elements.

Note that the array has the wrong element type: it should be int, not char. You should also check for array bounds in the first loop and for conversion success of the last scanf.

Here is a corrected version:

#include <stdio.h>

int countCopies(int a[], int n, int x);

int main(void) {
    int intArray[100]; //The integer array can only hold 100 integers
    int i, x;

    printf("Please enter a series of integers, end the list with the word end.\n");

    for (i = 0; i < sizeof(intArray) / sizeof(*intArray); i++) {
        if (scanf("%d", &intArray[i]) != 1)
            break;
    }
    if (scanf("%d", &x) == 1) {
        printf("too many numbers\n");
        return 1;
    }
    scanf("%*s");  /* skip the end word.  Note that any word is OK */

    printf("Enter x:");
    if (scanf("%d", &x) == 1) {
        printf("Copies = %d\n", countCopies(intArray, i, x));
    }
    return 0;
}

int countCopies(int a[], int n, int x) {
    int j, count = 0;

    for (j = 0; j < n; j++) {
        if (a[j] == x) {
            count++;
        }
    }
    return count;
}

I would suggest to calculate number of elements before `for` loop instead of calculating in each iteration . — ameyCU, Feb 27 '16 at 12:17
@chqrlie, please test your code before posting it. it doesn't work. I agree that the OP has done that mistake, but still mentioning it as correct code is wrong. — Box Box Box Box, Feb 27 '16 at 12:19
@ameyCU: good point, the condition should be as quick as possible to evaluate, but in this case, the `sizeof` expression is a compile time constant, no overhead at using it in the condition expression. — chqrlie, Feb 27 '16 at 12:20
Thank you for replying so quickly, it was indeed a stupid mistake. — umbraatrox, Feb 27 '16 at 12:38

score 1 · Answer 2 · answered Feb 27 '16 at 12:21

To expand on chqrlie's answer,

The code for (i = 0; i < n; i++) {/* do stuff */} executes in the following order:

The variable i is set to zero.
The conditional i < n is tested. If true, the code in /* do stuff */ is executed, otherwise, the loop is exited.
The increment i++ is applied.
Go back to 2.

That means that on the loop iteration when i gets incremented to n, the for loop breaks before the code inside the loop gets executed, so if you're accessing an array with n elements, the last element that gets accessed has the index n - 1. Hence the C loop paradigm that chqrlie mentioned.

Comparing arrays in a function

2 Answers2