6
\$\begingroup\$

I have implemented a dynamic array in C. I am a beginner in C so any constructive feedback about improving the implementation would be greatly appreciated.

Header file for the implementation( dyn_array.h)

#ifndef TYPE_H
#define TYPE int
#define TYPE_SIZE sizeof(int)
#endif

#ifndef STDDEF_H
#include <stddef.h>
#endif

#ifndef STDBOOL_H
#include <stdbool.h>
#endif

#ifndef INIT_BUFFER_SIZE
#define INIT_BUFFER_SIZE 2
#endif


typedef struct DynArray
{
        TYPE *data;
        size_t size;
        size_t capicity;
}DynArray;


bool DynArray_init(DynArray *self);
TYPE DynArray_getElement(const DynArray *self, size_t pos);
bool DynArray_setElement(DynArray *self, size_t pos, TYPE value);
size_t DynArray_getSize(const DynArray *self);
bool DynArray_pushBack(DynArray *self, TYPE value);
TYPE DynArray_removeElement(DynArray *self, size_t pos);

dyn_array.c

#include "dyn_array.h"
#include <stdint.h>
#include <stdbool.h>
#include <stdlib.h>
#include <string.h>
#include <stddef.h>
#include <assert.h>
#include <stdio.h>






/*Allocate an pool of memory to store data upto N elements
 * 
 * @param capicity
 *  capacity for the data pool 
 *
 * @returns 
 *  Pointer to a memory area of type TYPE with given number
 *  */


TYPE * __DynArray_createDataPool(size_t capicity)
{
    if (capicity != 0)
    {
        size_t bytesize =  TYPE_SIZE * capicity;
        TYPE *tmp = malloc(bytesize);
        if (!tmp)
            return NULL;
        tmp = memset(tmp, 0x00, bytesize);
        return tmp;
    }
    return NULL;
}

/*Initilize an DynArray
 * 
 * @param  self
 *      A pointer to an DynArray struct
 *
 * @returns
 *  true if initilization is successful or false if initlization is
 *  unsuccessful (possible reason - out of memory or bad pointer)
 *
 *  
 * */
bool DynArray_init(DynArray *self)
{
    if (self)
    {
        self->size = 0;
        self->data = __DynArray_createDataPool(INIT_BUFFER_SIZE);
        if (!self->data)
            return false;
        self->capicity = INIT_BUFFER_SIZE;
        return true;
    }
    return false;
}

/**
 *returns the element at a given index
 *
 * @param index
 *      index of the element that need to be read
 *
 * @returns
 *      value of the element at given index,
 *      assert Fails if the it's called with an invalid index
 *      and NDEBUG is not defined.
 *
 **/

TYPE DynArray_getElement(const DynArray *self, size_t index)
{
    assert(index < (self->size));
    return self->data[index];
}

/* double the capicity of an array
 *
 * */
bool __DynArray_expendArray(DynArray *self)
{
    if (self)
    {
        TYPE *tmp = __DynArray_createDataPool(2*(self->capicity));
        if (!tmp)
            return false;
        size_t byteToCopy = TYPE_SIZE* (self->size);
        tmp = memcpy(tmp, self->data, byteToCopy);
        free(self->data);
        self->data = NULL;
        self->data =  tmp;
        self->capicity = 2*(self->capicity);
        return true;

    }
    return false;
}

bool __DynArray_shrinkArray(DynArray *self, size_t capicity)
{
    TYPE *tmp = __DynArray_createDataPool(capicity);
    if (!tmp)
        return false;
    size_t byteToCopy = TYPE_SIZE*(self->size);
    tmp = memcpy(tmp, self->data, byteToCopy);
    free(self->data);
    self->data = tmp;
    self->capicity = capicity;
    return true;
}

/* push an element to last of the array
 *
 * @param self
 *      pointer to the DynArray struct
 *
 * @param value
 *      Value that need to be pushed
 *
 * @returns 
 *      true if push is successfule otherwise false
 * */

bool DynArray_pushBack(DynArray *self, TYPE value)
{
    if ((self->size) == (self->capicity))
    {
        bool res =  __DynArray_expendArray(self);
        if(!res)
            return false;
    }
    self->data[self->size] = value;
    self->size += 1;
    return true;
}
/*
 *
 * returns the current size of elements in array
 * @param self
 *      pointer to a DynArray struct
 *
 * @returns
 *      current size of the array
*/


size_t DynArray_getSize(const DynArray *self)
{
    return self->size;
}

/*remove the element at a given index
 *
 *@param self
 *      pointer to the DynArray struct
 *@param index
        index of the element that needs to be removed
        (If the index is greater then the element in array then the return value is undefined)
 *
 * @returns
 *      element that's is removed from the given index
 * */

TYPE DynArray_removeElement(DynArray *self, size_t index)
{
    assert(index < self->size);
    if (self->size < (self->capicity/4))
    {
        __DynArray_shrinkArray(self,(self->capicity/2));
    }
    TYPE indexValue = self->data[index];
    for (size_t i = index; i < (self->size - 1); i++)
        self->data[i] = self->data[i+1];
    self->size -= 1;
    return indexValue;
}
\$\endgroup\$
1
  • \$\begingroup\$ Welcome to code review. Hopefully you receive valuable feedback! \$\endgroup\$ Commented Apr 13, 2018 at 4:46

1 Answer 1

6
\$\begingroup\$

There are a couple small spelling errors that I noticed:

  • capacity, not capicity
  • expandArray, not expendArray

Include guard

Your header file lacks an include guard, you won't be able to include it more than once, and it will error if you attempt to do so. (Consider what happens if you include this header, then a second header, and the second header includes this header.)

At very top of your header you should add this:

#ifndef H_DYNARRAY
#define H_DYNARRAY

and last line must be:

#endif

This will protect it from being included more than once, so you can include "dyn_array.h" to your hearts content.

STDDEF_H and STDBOOL_H

No need to define these — standard headers already contain include guards.

calloc

Instead of malloc + memset, consider using calloc:

TYPE * __DynArray_createDataPool(size_t capacity)
{
    if (capacity == 0) {
        return NULL;
    }
    return calloc(capacity, TYPE_SIZE);
}

If allocation fails, calloc will return NULL.

There's a little more information about calloc here: https://stackoverflow.com/a/2688522

realloc

Same as above — instead of malloc + memcpy + free, consider using realloc instead. You can see man 3 realloc, but basically:

bool __DynArray_expandArray(DynArray *self)
{
    if (!self) {
        return false;
    }

    TYPE *tmp = realloc(self->data, TYPE_SIZE * self->capacity * 2);
    if (tmp == NULL) {
        return false;
    }

    // Fill new memory with zeros
    memset(tmp + self->capacity, 0, self->capacity);

    self->data = tmp;
    self->capacity *= 2;
    return true;
}

Note: realloc won't initialize new memory with zero's, so if that is important, you'll want to manually clear it out.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.