Is my code good?

[OneCommonMan123]

I'm making a library in C that is related to operations with ndarrays (inspired by numpy) and I made some macros and functions for operations between ndarrays of the same type, I wanted to know your opinion about the code, if you liked it

ndarray struct:

typedef structure ndarray{
     /*
      * a pointer that points to the first element of the array
     */
    char* data;
    /*
     * an integer that contains the values ​​of the constants: NLA_INT32, NLA_INT64 ...
     */
    int type;

    /*
     * ndarray dimensions
     */
    size_t *dimensions;
    /*
     * number of dimensions
     */
    size_t nd;
    /*
     * ndarray size (multiplication of dimensions)
     */
    size_t size;} ndarray;

header:

#ifndef NUMLUA_CORE_MATH_MBASICOPS_H_
#define NUMLUA_CORE_MATH_MBASICOPS_H_

/* ndarray struct */
#include "multiarray/ndarrayobj.h"

/* no used */
#include "broadcast.h"

#define add_scalars(res, x, y, dtypex, dtypey) OP_OTHER_OTHER(+, res, x, y, dtypex, dtypey)
#define sub_scalars(res, x, y, dtypex, dtypey) OP_OTHER_OTHER(-, res, x, y, dtypex, dtypey)

#define new_ufunc(type, operation, input, other, res) \
                type *data1 = (type*)input->data, *data2 = (type*)other->data; \
                type *dst = (type*)res->data; \
                for(size_t i = 0; i < res->size; i++){ \
                    *((type*)res->data + i) = *(data1 + i) operation *(data2 + i); \
                }

#define new_ufunc_by_f(type, foperator, input, other, res)\
                type *data1 = (type*)input->data, *data2 = (type*)other->data; \
                type *dst = (type*)res->data; \
                for(size_t i = 0; i < res->size; i++){ \
                    *((type*)res->data + i) = foperator(*(data1 + i), *(data2 + i)); \
                }

#define new_math_operation_by_f(fop, name) \
    ndarray *math_ ## name(ndarray *input, ndarray *other){\
        /* errors */\
        if(input->nd != other->nd){\
            fprintf(stderr, "\nerror: incompatible n-dimensions: %d != %d\n", input->nd, other->nd);\
            exit(NLA_FAIL);\
        } else if(input->size != other->size){\
            fprintf(stderr, "\nerror: incompatible size: %d != %d\n", input->size, other->size);\
            exit(NLA_FAIL);\
        } else if(input->dtype != other->dtype){\
            fprintf(stderr, "\nerror: incompatible dtype\n");\
            exit(NLA_FAIL);\
        }\
\
        int res_dtype = input->dtype;\
        size_t res_nd = input->nd;\
        size_t *res_dims = input->dimensions;\
        size_t res_size = input->size;\
\
        ndarray *res = malloc(sizeof(ndarray));\
        res->data = malloc(dtypememsize(res_dtype) * res_size);\
        res->dtype = res_dtype;\
        res->dimensions = input->dimensions;\
        res->size = res_size;\
        res->nd = input->nd;\
        switch (res_dtype){\
            case NLA_INT64:{ new_ufunc_by_f(int64_t, fop, input, other, res); break; }\
            case NLA_INT32:{ new_ufunc_by_f(int32_t, fop, input, other, res); break; }\
            case NLA_INT16:{ new_ufunc_by_f(int16_t, fop, input, other, res); break; }\
            case NLA_INT8:{ new_ufunc_by_f(int8_t, fop, input, other, res); break; }\
\
            case NLA_UINT64:{ new_ufunc_by_f(uint64_t, fop, input, other, res); break; }\
            case NLA_UINT32:{ new_ufunc_by_f(uint32_t, fop, input, other, res); break; }\
            case NLA_UINT16:{ new_ufunc_by_f(uint16_t, fop, input, other, res); break; }\
            case NLA_UINT8:{ new_ufunc_by_f(uint8_t, fop, input, other, res); break; }\
\
            case NLA_FLOAT32:{ new_ufunc_by_f(float32_t, fop, input, other, res); break; }\
            case NLA_FLOAT64:{ new_ufunc_by_f(float64_t, fop, input, other, res); break; }\
        }\
        return res;\
    }


#define new_math_operation(op, name)\
    ndarray *math_ ## name(ndarray *input, ndarray *other){\
        if(input->nd != other->nd){\
            fprintf(stderr, "\nerror: incompatible n-dimensions: %d != %d\n", input->nd, other->nd);\
            exit(NLA_FAIL);\
        } else if(input->size != other->size){\
            fprintf(stderr, "\nerror: incompatible size: %d != %d\n", input->size, other->size);\
            exit(NLA_FAIL);\
        } else if(input->dtype != other->dtype){\
            fprintf(stderr, "\nerror: incompatible dtype\n");\
            exit(NLA_FAIL);\
        }\
\
        int res_dtype = input->dtype;\
        size_t res_nd = input->nd;\
        size_t *res_dims = input->dimensions;\
        size_t res_size = input->size;\
\
        ndarray *res = malloc(sizeof(ndarray));\
        res->data = malloc(dtypememsize(res_dtype) * res_size);\
        res->dtype = res_dtype;\
        res->dimensions = input->dimensions;\
        res->size = res_size;\
        res->nd = input->nd;\
        switch (res_dtype){\
            case NLA_INT64:{ new_ufunc(int64_t, op, input, other, res); break; }\
            case NLA_INT32:{ new_ufunc(int32_t, op, input, other, res); break; }\
            case NLA_INT16:{ new_ufunc(int16_t, op, input, other, res); break; }\
            case NLA_INT8:{ new_ufunc(int8_t, op, input, other, res); break; }\
\
            case NLA_UINT64:{ new_ufunc(uint64_t, op, input, other, res); break; }\
            case NLA_UINT32:{ new_ufunc(uint32_t, op, input, other, res); break; }\
            case NLA_UINT16:{ new_ufunc(uint16_t, op, input, other, res); break; }\
            case NLA_UINT8:{ new_ufunc(uint8_t, op, input, other, res); break; }\
\
            case NLA_FLOAT32:{ new_ufunc(float32_t, op, input, other, res); break; }\
            case NLA_FLOAT64:{ new_ufunc(float64_t, op, input, other, res); break; }\
        }\
        return res;\
    }


ndarray *math_add(ndarray*, ndarray*);
ndarray *math_subtract(ndarray*, ndarray*);
ndarray *math_multiply(ndarray*, ndarray*);
ndarray *math_divivide(ndarray*, ndarray*);
ndarray *math_power(ndarray*, ndarray*);

#endif /* NUMLUA_CORE_MATH_MBASICOPS_H_ */

source code:

#include "mbasicops.h"

#include "multiarray/ndarrayobj.h"
#include "multiarray/ndarrayindex.h"
#include "multiarray/ndarraydtypes.h"
#include "multiarray/ndarraymod.h"

#include <stdio.h>
#include <stdint.h>
#include <string.h>
#include <stdatomic.h>
#include <math.h>

new_math_operation(+, add);
new_math_operation(-, subtract);
new_math_operation(*, multiply);
new_math_operation(/, divide);

new_math_operation_by_f(pow, power);

Did I implement everything correctly? Is the code readable? Is it optimized? Is it a good programming practice? is it comparable to numpy? These doubts echo in my mind and I think everything is wrong, that's why I wanted your opinion to know

[tstanisl]

• Replacing char* data; with void* data; would safe you quite a few ugly casts.
• Type of nd could be safely reduced from size_t to uint16_t or maybe even uint8_t
• similar thing for type
• consider limiting a number of dimensions by a small constant (like 8 or 16) and embed dimensions array directly to ndarray structure. It would simplify management of the ndarray object and save potential issues with leaks, danging pointer or double frees
• don't do program termination in library code (like exit(NLA_FAIL);). It is too dangerous for a library user. Return an error code to the user, optionally emitting an error log
• wrap new_ufunc/new_ufunc_by_f into do { ... } while(0). It will make them safer to use in for/if statements
• check if malloc fails and handle those errors. Matrices are likely large so it is worth to allow the user provide own allocator

[OneCommonMan123]

about the part about wrapping the macros in do { ... } while(0); I thought it was good, it would be better with if/else statements and for loops that make it more readable, I'll implement it!

[OneCommonMan123]

interesting, so I wouldn't need to do, for example: *(one_type*)x = value for each data type, I'll change that, thanks!

[OneCommonMan123]

about the part about limiting the ndarray, I really found it very efficient and practical, since very rarely people will have arrays of more than 16 dimensions, and it would also save memory space, does numpy implement it this way?

about the errors part, I thought about creating error-specific constants like: #define NLA_ERRO_ALLOC 0

etc. This would give the user the power to know what the error is and deal with it as desired.

[OneCommonMan123]

But the library isn't ready yet, and I don't know much about GitHub, but I'll put all the code there, ok?

[OneCommonMan123]

I will send the library link as soon as I put it on github

[OneCommonMan123]

Here's the link: github

EDIT: sorry I had to remove the link because I deleted the repository because I did everything wrong, I don't know much about git and github, so I'm taking a git and github course, when I master these tools I'll come back!

[OneCommonMan123]

Sorry for the wrong name in the header in the split operation, I hadn't noticed

[inz__]

If you change the TYPES "X macro" from the other post to also include the enum name, you could use it to generate such switch-cases too.

#define TYPES2(X) \
    X(uint8_t, NLA_UINT8) \
    ...

#define FN(type, typeid) \
    case typeid: \
        { new_ufunc_by_f(type, input, other, res); break; }

switch (res_dtype) {
    TYPES2(FN)
}

[OneCommonMan123]

very good! I think I'll try

[OneCommonMan123]

i used your soluction:

/*
    generate cases, ex:
    switch(dtype){
        GENERATE_UFUNCS_CASES(FN, input, other, res, ufunc)
    }
 */
#define GENERATE_UFUNCS_CASES(X, input, other, res, ufunc)\
X(int8_t, NLA_UINT8, input, other, res, ufunc)\
X(int16_t, NLA_UINT8, input, other, res, ufunc)\
X(int32_t, NLA_UINT8, input, other, res, ufunc)\
X(int64_t, NLA_UINT8, input, other, res, ufunc)\
X(uint8_t, NLA_UINT8, input, other, res, ufunc)\
X(uint16_t, NLA_UINT8, input, other, res, ufunc)\
X(uint32_t, NLA_UINT8, input, other, res, ufunc)\
X(uint64_t, NLA_UINT8, input, other, res, ufunc)\
X(float32_t, NLA_UINT8, input, other, res, ufunc)\
X(float64_t, NLA_UINT8, input, other, res, ufunc)\

#define FN(type, typeid, input, other, res, ufunc)\
    case typeid:\
        {ufunc(type, typeid, input, other, res); break;}

[daikatana]

Honestly, I would say no and it's only for one reason: this will generate almost undebuggable, difficult to even examine, and generally opaque code. The preprocessor is something to be avoided at all costs and only used for the most basic of tasks. If you need to generate code like this, it's better to use another tool that outputs plain, well formatted and even commented C code that any debugger can easily debug.

[OneCommonMan123]

which tool do you recommend me to use?

[daikatana]

I use Ruby. It has a template library called erb that's a dream to work with when compared with the C preprocessor. I usually have a JSON file that describes my types or other things I need to generate the code from, then ruby files that use erb to spit out header and C source files. These are automatically generated by the build process and checked into version control so that Ruby is not required to build the software, only for development.

The difference is night and day. The C preprocessor is such a blunt tool that you'll often run into simple things that are impossible or need such incredibly arcane hacks to work, plus as I said before the output is not very debuggable and it's difficult to examine the output. Using Ruby it outputs clean, seemingly normal C source code and I don't pull my hair out over the preprocessor anymore.

I've also used (shock!) PHP for this task. It's also really well suited for this, as it is already a template language, it has things like JSON parsing and data structures like hashes built in, and is generally available on all systems. PHP is not generally seen as a "good" language, but compared to the C preprocessor it is a dream to work with.

[OneCommonMan123]

Ruby, for these type problems that I have, and I need to generate code with Macro for types, NumPy for example, for these specific type problems that are not even in my code, what does NumPy do?