C101: the best way to fill an array from user input?

Question

I'm having a hard time understanding and therefore managing arrays and indexes manually in C. These are my two classic approaches but they doesn't seem to work as they keep looping when the condition is reached:

#include<stdio.h>
#define MAX 255

int main(){

    int arr[MAX]={0};
    int idx=0;

    /* Approach #1 */

    printf("Enter elements, -1 to finish:\n");
    scanf("%d", &arr[idx]);

    while(arr[idx-1] != -1 && idx < MAX){
        printf("Enter elements, -1 to finish:\n");
        scanf("%d", &arr[idx]);
        idx++;        
    }

    /* Approach #2 */

    do{
        printf("Enter elements, -1 to finish:\n");
        scanf("%d", &arr[idx]);
        idx++;
    }while(arr[idx-1] != -1 && idx < MAX);

    // Main func continues here.

}

Any advice would be much appreciated!

Update:

Now it works! thanks MUCH all of you guys for such instant responses. This is definitely a great community, it's helping me a lot.

Answer 1

arr[idx] <= MAX

should be

idx <= MAX

Answer 2

C arrays begin counting from 0.

If you allocate an array of size MAX, accessing the element at MAX would be an error. Change the loop to;

int arr[MAX];
for ( .... && idx < MAX )

Answer 3

arr[idx] <= MAX

should be

idx < MAX

unless you are checking the item instead of the index.

You are also always checking the "next" element for -1 (arr[idx] != -1) because you are incrementing idx prior to checking your added value.

so if you had

arr[idx-1] != -1

you would be fine.

Answer 4

In your first while loop, the

arr[idx] <= MAX

line should read

idx <= MAX

In your second loop, you're incrementing idx before the test - it should end with

} while ((arr[idx-1] != -1) && (idx-1 <= MAX));

I also tend to parenthesize all internal conditions just to be absolutely certain that the precedence is correct (hence the extra brackets above).

Answer 5

while(arr[idx] != -1 && idx <= MAX){ // Fixed by sklivvz
    printf("Enter elements, -1 to finish:\n");
    scanf("%d", &arr[idx]);
    idx++;        
}

First of all, you should check if the index variabel idx is smaller than MAX (not smaller than or equal to). You would go out of bounds if your index equals MAX. An array with MAX = 10 has index values 0 to and including 9, but not 10.

Secondly, you add the first element to arr[0], you increment your index from 0 to 1, then you jump back to the while-condition and check if arr[1] == -1, which it is not. So check instead that arr[idx-1] != -1. Note however that the first time you get to the while-loop, you will actually check arr[-1] != -1, which is also out of bounds. ;) So you need to figure out how to solve this one.

Answer 6

I'd go with somthing like this.

You don't have to worry about array bounds and other confusing conditions.

int cnt = MAX;        // how many elements in the array, in this case MAX
int * p = &arr[0];    // p is a pointer to an integer and is initialize to the address of the first
                      // element of the array. So now *p is the same as arr[0] and p is same as &arr[0]

// iterate over all elements. stop when cnt == 0
while (cnt) {

    // do somthing
    scanf("%d", *p); // remember  that *p is same as arr[some index]
    if (*p == -1)    // inspect element to see what user entered
        break;

    cnt --;  // loop counter
    p++;     // incrementing p to point to next element in the array
}

Answer 7

to Roman M:

First of all, the guy asking the question has just started a programming course, and has probably not yet learned about pointers. Secondly, you now deal with both a counter and a pointer. I'm not sure I see the benefit of doing that versus using an index like this:

for(idx=0; idx < MAX; ++idx) {

scanf("%d", &arr[idx]);
if(arr[idx] == -1)
    break;

}

Answer 8

Using a for loop you can eliminate the need for the messy idx-1 checking code:

/* Approach #3*/
int i;
int value;

for (i = 0; i < MAX; ++i)
{
  printf("Enter elements, -1 to finish:\n");
  scanf("%d", &value);
  if (value == -1) break;
  arr[i] = value;
}