C inner product function without using array subscripting

Question

As part of a question designed to help us understand the relationship between pointers and arrays in C, I've been asked to write an inner product function that doesn't use any array subscripting. Here's what I came up with, but it looks like the kind of complicated 'clever' coding that we've traditionally been told NOT to write. Any feedback on it, or how it could be done better / more efficiently would be much appreciated.

int inner_product(const int *a, const int *b, int n) { const int *p, *q; int result = 0; for(p = a, q = b; p < a + n, q < b + n; p++, q++) { result += *p * *q; } return result; } 

Edit - Inner product is simply summing the product of the indexes, so a[1,2,3] b[2,3,4] would be 1*2 + 2*3 + 3*4

Answer 1

I have a few comments on style. (By which I really mean, 100% opinion... :D)

---

One declaration per line

A lot of people strongly disagree with this, but I find it much easier to read code that has one declaration per line.

const int *p; const int *q; 

Is a lot clearer to me than:

const int *p, *q; 

Though really, the only time I've vehemently opposed to it is if both variables are not the same in terms of a pointer or value:

const int *p, q; 

Is a harder to read and more prone to errors than:

const int *p; const int q; 

---

n

I would name the n paramter something more meaningful. I would consider calling it size or num.

It's fairly apparent that n is meant to be the number of elements, but two or three extra keystrokes seems worth the extra clarity.

---

Possible implementation

I would probably write the function something like this:

int inner_product(const int *a, const int *b, int num) { const int* end = a + num; int sum = 0; while (a != end) { sum += (*a) * (*b); ++a; ++b; } return sum; } 

Since the two pointers are incremented at the same time, there's no reason to check both.

This should be a tiny bit more efficient (well... probably not once the compiler does it's magic optimisations on the original), and I think it's a bit clearer.

Answer 2

Just a point, that code will error on compilation: p++ and q++ are not permitted operations if you have a const int.

Now, to write it more cleanly (I know next to nothing about inner products, and google hasn't been much help):

int inner_product(const int *a, const int *b, int n) { int *p = a, *q = b; int result = 0; while( (p < a + n ) && (q < b + n ) ) /*|| might be required here, but a comma probably (never come across it) won't work*/ { result += *p * *q; p++; q++; } return result; } 

The if loop you used was messy, hard to read, and too 'clever'. This way, it is clear what your code is doing. To be even more easy to read, I'd surround my calls to the pointers *p in parenthesis: (*p), but that's a matter of personal taste.

Edit: Since the definition of inner product has now been defined for me, I can safely say that your algorithm is the most clean I could have come up with, and is the fastest by far (linear time).

EDIT: Where I say int *p = a, *q = b;, it would be more proper to say const int *p = a, *q = b;

Answer 3

I agree with all the points from Corbin but i personally would use a simple for loop instead as this is one of the more common ways to iterate through arrays:

int inner_product(const int *a, const int *b, int size) { int sum = 0; int i = 0; for (i=0; i<size; ++i) { sum += *(a+i) * *(b+i); } return sum; }