Product of all but one number in a sequence - Follow Up (2)

Question

This code is a improved version of implementation which asked for a review: Product of all but one number in a sequence.

#include <stdio.h>
#include <stdlib.h> 
//without division, with O(n) time, but extra space complexity as suggested
//return new array on the heap 
int *derive_products(const int *nums, int count)
{
 int *retval = malloc(count * sizeof *retval);
 if (!retval && count)
 return 0;
 
 int mult=1; //product of prefix or suffix elements
 
 //swipe up 
 for(int i=0; i<count; i++) {
 retval[i] = mult;
 mult *= nums[i];
 }
 
 //swipe down
 for(int j=count-1, mult=1; j>=0; j--) {
 retval[j] *= mult;
 mult *= nums[j];
 }
 
 return retval;
}
int main()
{
 /*Given an array of integers, return a new array such that each element at index i of the 
 new array is the product of all the numbers in the original array except the one at i.
 For example, if our input was [1, 2, 3, 4, 5], the expected output would be 
 [120, 60, 40, 30, 24] */
 int nums[] = {1, 2, 2, 4, 6}; 
 int size = sizeof(nums)/sizeof(nums[0]);
 
 int *products = derive_products(nums, size); //get a new array
 
 for (int i = 0; i < size; i++) 
 printf("%d ", products[i] ); 
 
 free(products); //release heap memory
 
}

• chux: Rather than "/return new array on the heap ", get right to the point as C does not define "heap". Instead. "// Returns allocated array"/
• chux: Why does code describe the output of a commented test case and then code uses a test with another set of data? I'd expect the true test case to have a commented expectation - or a run-time check.
• Erdenebat Ulziisaikhan: @chux-ReinstateMonica I declared random int array since I only focus on product generator function. But I got the point thanks.

Answer 1

Proper scoping

int mult=1; //product of prefix or suffix elements

1. mult above is only used in the first loop. The one used in the second loop is defined there, and limited to that loop. This one should be the same.

Proper naming

2. j in the second loop is an unexpected name. As it is an outer loop, you should use i (again).

int size = sizeof(nums)/sizeof(nums[0]);

3. While size isn't really wrong, I would expect a different unit of measurement (namely bytes, as for sizeof, instead of elements) considering the name. I suggest going for count.

Check for failure

int *products = derive_products(nums, size); //get a new array

4. The above can fail, in which case it returns null. Don't just assume success.

Comments

//without division, with O(n) time, but extra space complexity as suggested
//return new array on the heap 

5. Comments should be to the point, omit the obvious, and describe the motivation.

   The above could be:

   // without division in O(n) time
   // free(result)
   

6. Yes, writing down the task as a comment is a good idea. But if you have test-data, just write a test.

7. Your comment after a line of code are pretty superfluous. Are you forced to sprinkle more comments?
   See Guessing a number, but comments concerning

Whitespace

8. Your use of whitespace is inconsistent. Do you surround binary operators with single whitespace?
   Sometimes you do, sometimes you don't. Imho, doing so looks better, so standardise on that. There are plenty automatic code formatters, so you don't even have to do the hard work yourself.

• chux: Good point about scoping. It took a while to see that mult declaration in int j=count-1, mult=1;. for body certainty looks like it is using the prior code's mult.