-1

can someone please help me figure out what i'm doing wrong here? i'm getting inaccurate results here. I seem to be getting the first value in the array each time and i cant seem to figure out what i'm doing incorrectly

#include <stdio.h>

int getbillsum ( int price[] );

int main( void )
{
    int itemprice [10];
    int total = 0;
    for (int c=0;c <10;c++ ) //Looping to get item prices
    {
        printf ("\nEnter the price of the item: ");
        scanf (" %d", &itemprice[c]);   
    }
    total = getbillsum (itemprice);
    printf ("%d", total);
    return 0;
}

int getbillsum (int price []) //function to sum the values in array
{
    int sum = 0;
    for (int i=0; i<sizeof(price); i++) 
    {
        sum+=price[i];
    }
    return sum;
}
Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
  • 2
    Welcome to Stack Overflow. Please read the [About] page soon. The code you show doesn't compile (`price` is not a known type, so `int getbillsum(price);` won't compile). This makes people leery of helping you. Make sure your code compiles (or describe why it won't and why you can't fix it so that it does). – Jonathan Leffler Jan 21 '14 at 07:33
  • Sizeof isn't doing what you are expecting here. It's using the variable name which is a pointer to the first element. There is no way to get size of an array by just passing it a pointer so instead pass it a limit for the number of items to consider. – Chris Zhang Jan 21 '14 at 07:33
  • You seems not paying any effort to debug your own code, at least passing 2 parameter into function which accept only 1 is very obvious problem. – moeCake Jan 21 '14 at 07:35

3 Answers3

3

You can't pass arrays to functions in C (well, not as an array anyway). Arrays decay into pointers, the sizeof which is always the same (4 for 32 bit systems, 8 for 64 bits).
For more information see paragraph 2.3 here.

The easiest, most common and most reliable way of solving your issue is to pass the length of the array as a second argument:

int getbillsum (int *price, size_t len)
{
    int sum = 0;
    for (int i=0; i<len; ++i)
        sum += price[i];
    return sum;
}
//usage
int main ( void )
{
    int price[10];
    for(int i=0;i<10;++i)
        scanf(" %d", &price[i]);
    printf("Sum: %d\n", getbillsum(price, sizeof(price)/sizeof(*price)));
    return 0;
}

You also had a problem in your code: you added the return statement inside of your loop.
Just a quick-tip: The sum of an array of ints is not unlikely to be too much for a single int to hold, so I'd change the return-type of getbillsum to long, too

I've also edited your question, addressing quite a lot of issues considering how short your code was:

int getbillsum ( price );//missing type, changed to
int getbillsum ( int price[] );//better would be int getbillsum ( int *price ); but considering your question, left it as array
scanf ("%d", &itemprice[c]);//unsafe, changed it to
scanf (" %d", &itemprice[c]);//add space
total = getbillsum (itemprice,9);//why the second param?
total = getbillsum (itemprice);//to match function prototype
return sum;//moved OUTSIDE of the loop...
Elias Van Ootegem
  • 74,482
  • 9
  • 111
  • 149
  • @user3218106: You're welcome... please do read the help section of this site, as one of the comments already pointed out. Posting comments saying _thank you_ is discouraged. The _"proper"_ way of thanking someone... and if you do, there's a +2 rep bonus in it for you :-P – Elias Van Ootegem Jan 21 '14 at 07:59
1

sizeof(price) does not give you the length of the array, but the size of the pointer (int price[]), which is probably 4. Also, you immediately return in the first for run. Put return outside the for loop.

You do fix it by supplying the array size, but you never use it. Update your getbillsum function:

int getbillsum (int price [], int length) //function to sum the values in array
{
   int sum = 0;
   for (int i=0; i<length; i++) 
   {
     sum+=price[i];
   }
   return sum;
}
Bart Friederichs
  • 33,050
  • 15
  • 95
  • 195
  • Is `size` the _mot juste_? It should be number. The size would be the number of bytes in the array, so you'd have to scale by the size of an entry in the array. – Jonathan Leffler Jan 21 '14 at 07:35
  • @JonathanLeffler I have seen both `size` and `length` in these cases (ref C#, Java). – Bart Friederichs Jan 21 '14 at 07:37
  • C# and Java have different considerations, and in any case you'd be able to pass the array and access its `.length()` member function — at least, IIRC, that's how Java does it; I don't know about C#, but it is likely similar. In any case, the rules are different in C. I'd go with `length` as preferable to `size` because in C, size has connotations of `sizeof()` and `size_t` and byte counts. – Jonathan Leffler Jan 21 '14 at 07:40
  • @JonathanLeffler: I don't see any problem with passing length as a `size_t` type, seeing as an array can't be `-1` long, I'd even say an unsigned type is preferable. That's why I used `size_t` in my answer... would it be better, then, if I changed it to `unsigned int` or something? – Elias Van Ootegem Jan 21 '14 at 07:45
  • @JonathanLeffler I am all for the semantics, less for language considerations, but I agree with you. I'll update my answer. – Bart Friederichs Jan 21 '14 at 08:02
0

In addition to posted answers, you can consider a technique suggested in this answer.

Edit Quoted from comment

it's non-standard, dangerous (think of overflow, forgetting to dereference at the correct offset and the like), and you should not try this

In your case it will be something like that :

void *p = calloc(sizeof(itemprice) + sizeof(unsigned long int),1));
*((unsigned long int*)p) = 10;

quote from linked answer

n is now stored at ((unsigned long int)p)

Your getbillsum will look like that now (did not compile it, consider it as pseudocode)

 int getbillsum (void* p)
 {
     int* price = p+sizeof(unsigned long int);
     unsigned long int size = *p;
     int sum = 0;

     for (int i=0; i<size; i++) 
     {
        sum+=price[i];
     }
     return sum;
 }
Community
  • 1
  • 1
Dabo
  • 2,371
  • 2
  • 18
  • 26
  • Please, read the linked answer thoroughly, first: this _"solution"_ depends on non-standard behaviour, and therefore might not compile at all... it's a hack, that is _completely unnecessary_ What's so hard about keeping track of the size on-the-fly? Or even passing the length @ calltime? – Elias Van Ootegem Jan 21 '14 at 08:06
  • Perhaps, but SO aims to be a repo of Q&A formatted relevant, valid and _correct_ code... your answer just states _"you could do this"_ without it mentioning the fact that it's non-standard, dangerous (thing of overflow, forgetting to dereference at the correct offset and the like), and you _should not try this_. The only thing this approach is good for is to check if you understand pointer arithmetic. – Elias Van Ootegem Jan 21 '14 at 08:22
  • @EliasVanOotegem Warning added – Dabo Jan 21 '14 at 08:37