2

I would like to define a custom structure that contains two dynamically allocatable integer arrays a and b. In order to allocate memory for the arrays and initialise arrays with values I have written a "constructor" function initp. My approach is presented below.

A custom structure called pair:

typedef struct {
    ...
    int  l;  // length of compositions
    int  k;  // no. of elements in compositions
    int *a;  // composition 1
    int *b;  // composition 2
} pair;

A function to initialise custom structure:

int initp(..., int l, int k, int a[], int b[], pair *f) {
    ...
    f->l = l;
    f->k = k;

    // allocate composition arrays
    f->a = (int *) calloc(l, sizeof(int));
    f->b = (int *) calloc(l, sizeof(int));

    // initialise composition arrays
    for (int i = 0; i < k; i++) {
        f->a[i] = a[i];
        f->b[i] = b[i];
    }
}

A function call in the main program:

// initialise pairs
pair f1, f2;
initp(..., 10, 2, (int[]){3, 4}, (int[]){7, 6}, &f1);
initp(..., 10, 1, (int[]){4},    (int[]){9},    &f2);

I'm on a mission to write an "elegant" code. Therefore, the questions I have are:

  1. Is it possible to avoid specifying the no. of elements in arrays a and b passed to the initp? This is the parameter k. In the above examples it is 2 and 1.
  2. Is it possible to avoid "explicit" casting with (int[]) in the function call?
  3. Please let me know, if you have general comments and criticism on improving the quality of the code.
mabalenk
  • 887
  • 1
  • 8
  • 17
  • 2
    The arrays you pass to `initp` will decay to pointers to their first elements. And once you have a pointer to the first element of an array, that pointer is all you have, you lose all and any information the compiler might have had for the array and its size. And that goes for all pointers, it's technically a pointer to a *single* value. With a pointer alone you don't know if it's a pointer to that single value only, or a pointer to the first element of an array. So you *always* need to specify the length of all arrays. – Some programmer dude Oct 25 '18 at 16:34
  • 1
    And no you can't get rid of the "cast", that is needed for all [compound literals](https://en.cppreference.com/w/c/language/compound_literal). – Some programmer dude Oct 25 '18 at 16:35
  • 1
    OT: regarding: `f->b = (int *) calloc(l, sizeof(int));` and similar statements: 1) always check (!=NULL) the returned value to assure the operation was successful. 2) The returned type is `void*` which can be assigned to any pointer. Casting just clutters the code, making it more difficult to understand, debug, etc – user3629249 Oct 25 '18 at 16:36
  • 2
    Lastly the third point: Better naming of the variables would be nice. And you shouldn't really [cast the result of `malloc` (or its siblings)](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc). – Some programmer dude Oct 25 '18 at 16:36
  • 3
    If there's no problem with code I think it's OT for this site. Maybe https://codereview.stackexchange.com/ is a better fit – Support Ukraine Oct 25 '18 at 16:58

1 Answers1

1

Is it possible to avoid specifying the no. of elements in arrays a and b passed to the initp?

No.

Alternative: Create a macro INITP(l, a, b, f) with macro magic determines the a, b are truly arrays, their array element counts, insure their counts are equal and then calls initp().

I am not fan of this approach, but it is do-able - with limitations.

Note: for size information, consider size_t vs. int.


Is it possible to avoid "explicit" casting with (int[]) in the function call?

With (int[]){3, 4}, (int[]) is not a cast. It is simply the form of a compound literal and is required to form one.


if you have general comments and criticism on improving the quality of the code.

  1. For working code, consider Code review for a deeper review.

  2. I'd assign each array in turn - usually as fast or faster than interlacing.

     memcpy(f->a, a, sizeof f->a[0] * k);
     memcpy(f->b, b, sizeof f->b[0] * k);
    
  3. Handle allocation failures.

  4. Create a initp() companion like void uninitp(pair *f).

  5. Improve allocation for clarity, review and maintenance.

    // f->a = (int *) calloc(l, sizeof(int));
    f->a = calloc(l, sizeof f->a[0]);
    
  6. const, restrict considerations.

Hmmm, this is getting to be too full a review for here, so will end now.

chux - Reinstate Monica
  • 143,097
  • 13
  • 135
  • 256
  • Could you please elaborate on your comments regarding the use of `size_t` vs `int` and `const`, `restrict`? – mabalenk Oct 26 '18 at 16:32
  • 1
    @mabalenk Those questions are more appropriate for a code review. For now, ask yourself why use type `int` for an array size versus `long`, `unsigned`, `char`, etc. vs [`size_t`](https://en.wikipedia.org/wiki/C_data_types#stddef.h)? – chux - Reinstate Monica Oct 26 '18 at 16:54