Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Although I know I can use calloc then realloc to make a dynamic array in C, and that I also need a variable to keep track of the number of elements in the array, that isn't too productive to me. Coming from an object oriented background using other high level languages (Namely Java and Python), I decided that an arraylist would fit my needs. In C ofcourse, that'd mean that behind the curtains, I'd probably be using a dynamic array which I'd be reallocing every time I add a member to it, that member would be a pointer to another variable.

The Header

#ifndef _ARRAYLIST_H
#define _ARRAYLIST_H

typedef struct _arraylist ArrayList;

extern ArrayList *arraylist_create();
extern void arraylist_set_data(ArrayList *, void **);
extern void arraylist_add(ArrayList *, void *);
extern void *arraylist_get(ArrayList *, int);
extern void arraylist_remove(ArrayList *, int);
extern void arraylist_clear(ArrayList *);
extern void arraylist_deallocate(ArrayList *);

#endif /* _ARRAYLIST_H */

The Source

#include <stdlib.h>
#include <assert.h>
#include "ArrayList.h"

struct _arraylist {
    size_t size;
    size_t data_size;
    void ** data;
};

struct _arraylist *arraylist_create() {
    /* Allocate Memory */
    struct _arraylist *list = malloc(sizeof(struct _arraylist));
    assert(list != NULL);
    list->size = 0;
    list->data_size = 0;
    list->data = calloc(2, sizeof(void *));
    assert(list->data != NULL);
    list->data[0] = NULL;
    return list;
}

void arraylist_set_data(struct _arraylist *list, void ** data) {
    arraylist_clear(list);
    list->data = data;
    for(int i = 0; ( list->data[i] != NULL ); ++i) {
        list->data_size += sizeof(*list->data[i]);
        ++list->size;
    }
}

void arraylist_add(struct _arraylist *list, void *elem) {
    list->data_size += sizeof(*elem);
    void ** new_data = realloc(list->data, (list->size + 1) * (list->data_size));
    assert(new_data != NULL);
    new_data[list->size] = elem;
    new_data[list->size + 1] = NULL;
    arraylist_set_data(list, new_data);
    ++list->size;
}

void *arraylist_get(struct _arraylist *list, int index) {
    return list->data[index];
}

void arraylist_remove(struct _arraylist *list, int index) {
    list->data_size -= sizeof( *arraylist_get( list, index ) );
    for ( int i = index; list->data[i + 1] != NULL; ++i )
        list->data[i] = list->data[i + 1];
    list->data[list->size] = NULL;
    void ** new_data = realloc(list->data, list->size * list->data_size);
    assert(new_data != NULL);
    arraylist_set_data(list, new_data);
    --list->size;
}

void arraylist_clear(struct _arraylist *list) {
    /* free(list->data); */ /* Problem here */
    list->size = 0;
    list->data_size = 0;
    list->data = NULL;
}

void arraylist_deallocate(struct _arraylist *list) {
    arraylist_clear(list);
    free(list);
}

The code works fine, but there are a few problems I've faced so far.

  • If I want to add a member, I can't initialize a variable immediately, because I have to pass a pointer to an existing variable.

  • The first line in the arraylist_clear function causes a problem when running the problem, and without it (as valgrind says) I have a slight memory leak that I don't know how to deal with.

  • The arraylist_remove function doesn't work.

share|improve this question

closed as off-topic by syb0rg, Marc-Andre, JaDogg, Corbin, Kid Diamond Sep 30 '14 at 21:36

This question appears to be off-topic. The users who voted to close gave this specific reason:

  • "Questions containing broken code or asking for advice about code not yet written are off-topic, as the code is not ready for review. Such questions may be suitable for Stack Overflow or Programmers. After the question has been edited to contain working code, we will consider reopening it." – syb0rg, Marc-Andre, JaDogg, Corbin, Kid Diamond
If this question can be reworded to fit the rules in the help center, please edit the question.

1  
I'm afraid that since your arraylist_remove function doesn't work that this question is considered off-topic. –  syb0rg Sep 30 '14 at 19:18

1 Answer 1

up vote 3 down vote accepted

list->size and list->data_size are maintained in a lockstep, hence one of them is redundant and should be gone.

Even more redundant is a terminating NULL. Besides of additional maintenance it requires, it effectively prohibits the client to store a legitimate NULL. The arraylist_set_data should be passed a data size instead of relying on a terminating NULL. The arraylist_remove loop should use list->size as a terminating condition.

list->size * list->data_size in reallocs doesn't sound right. I suppose you meant list_size * sizeof(*list->data) or list->data_size.

I don't know what kind of problem free(list->data) causes, and on which circumstances. It is mandatory to have it though; valgrind is correct. Time to use a debugger I suppose.

share|improve this answer
    
That's good, you almost solved most of my problems. But still, why is arraylist_remove not working ? –  Amr Ayman Sep 30 '14 at 19:55
    
After Debugging, my implementation is now working fine! –  Amr Ayman Oct 1 '14 at 13:23

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