Return Styles: Pseud0ch, Terminal, Valhalla, NES, Geocities, Blue Moon. Entire thread

[CodeCritique] Criticize my Linked List

Name: Anonymous 2011-08-19 20:33

I have implemented linked lists in C as a learning exercise.  I'm a noob ass so tell me how my implementation sucks and I will fix it.  I didn't have a particular use-case in mind when I wrote it.

Header:
// llist.h
#ifndef LLIST_H_
#define LLIST_H_

struct LList;
typedef struct LList LList;

struct LLNode;
typedef struct LLNode LLNode;

struct LList {
    LLNode *front, *back;
};

struct LLNode {
    void *data;
    LLNode *next;
};

LList* llist_new(LList **self);
void llist_add(LList *self, void *v);
void llist_remove(LList *self, LLNode *node);
void llist_insert(LLNode *node, void *v);
void llist_destroy(LList *self);

#endif


C file:
// llist.c
#include <assert.h>
#include <stdlib.h>

#include "llist.h"

// Initialize a new, empty LList, in dynamically allocated memory.
LList *llist_new(LList **self) {
    *self = malloc(sizeof (LList));
    if (self == NULL) { return NULL; }

    (*self)->front = NULL;
    (*self)->back = NULL;

    return *self;
}


// Append a value to the LList.  Creates a new node.
void llist_add(LList *self, void *v) {
    LLNode *node = malloc(sizeof (LLNode));
    assert(node != NULL);

    node->data = v;
    node->next = NULL;

    // Put this node at the back of the list.
    if (self->front != NULL) {
        self->back->next = node;
        self->back = node;
    } else {
        self->back = node;
        self->front = node;
    }
}


// Iterate over the LList to locate a node, and then remove it from the list.
void llist_remove(LList *self, LLNode *node) {
    LLNode *iter_node = self->front;
    LLNode *prev_node = NULL;

    while (iter_node != NULL) {
        if (iter_node == node) {
            if (prev_node != NULL) { prev_node->next = iter_node->next; }
            else { self->front = NULL; }
            free(iter_node);
            return;
        }
        prev_node = iter_node;
        iter_node = iter_node->next;
    }

}


// Insert a value after a particular node.
void llist_insert(LLNode *node, void *v) {
    LLNode *next = node->next;
    LLNode *new_node = malloc(sizeof (LLNode));
   
    new_node->data = v;
    new_node->next = next;
   
    node->next = new_node;
}


void llist_destroy(LList *self) {
    LLNode *iter_node = self->front;

    while (iter_node != NULL) {
        LLNode *next = iter_node->next;
        free(iter_node);
        iter_node = next;
    }

    free(self);
}


Testcase:
// llist_test.c
#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#include "llist.h"

#ifdef _WIN32
char *strdup(char *s);
#endif

int main(void) {
    LList *phone_nums;
    LLNode *iter;

    if (llist_new(&phone_nums) == NULL) { printf("Out of memory."); exit(1); }
    llist_add(phone_nums, strdup("123-4567")); // Yes, I should free() these later...
    llist_add(phone_nums, strdup("890-1234"));
    llist_add(phone_nums, strdup("567-8901"));
    llist_add(phone_nums, strdup("234-5678"));
       
        LLNode *temp = NULL;
    iter = phone_nums->front;
    while (iter) {
        printf("%s\n", (char*)iter->data);
        if (!strcmp("890-1234", (char*)iter->data)) { temp = iter; }
        iter = iter->next;
    }
    puts("");
   
    llist_insert(temp, strdup("BEEP BOOP"));
   
    iter = phone_nums->front;
    while (iter) {
        printf("%s\n", (char*)iter->data);
        if (!strcmp("567-8901", (char*)iter->data)) { temp = iter; }
        iter = iter->next;
    }
    puts("");
   
    llist_remove(phone_nums, temp);
   
    iter = phone_nums->front;
    while (iter) {
        printf("%s\n", (char*)iter->data);
        if (!strcmp("567-8901", (char*)iter->data)) { temp = iter; }
        iter = iter->next;
    }
   
   
    llist_destroy(phone_nums);
    return 0;
}

#ifdef _WIN32
char *strdup(char *s) {
        char *p = malloc(strlen(s) + 1);
        strcpy(p, s);
        return p;
}
#endif

Name: Anonymous 2011-08-21 19:30

*when*

Name: Anonymous 2011-08-21 19:37

>>79
BTW, my whole beef was that while the code was written in C, it wasn't C (in the strictest sense).
I hate you, Zhivago, I truly do.

Name: Anonymous 2011-08-21 21:03

>>82
Why flatter him by comparing him to Zhivago?  This guy is an idiot.

Name: Anonymous 2011-08-21 21:26

>>79
No, I was asking that if I explicitly copy and paste the header file into the source file and that the header file is valid ANSI-C if then you might accept the entire file as C?

Name: Anonymous 2011-08-21 21:30

>>83
Why because I actuall know what I'm talking about you mental midget? Maybe you should actually take a look at the ANSI/ISO C standard instead of trying to argue with someone who does this shit for a living. Now shut up and go clean another toilet.

Name: i_have_a_raging_boner !!kCq+A64Losi56ze 2011-08-21 21:35

>>84
No matter how you do it, nonstandard C functions are nonstandard C functions.  I would say look at how some of the more popular pieces of open source software handles nonstandard C functions. But I'm afraid that you, along with the other hoshead that is calling me an idiot, wouldn't be able to tell what the programmers do in cases like this.

Name: Anonymous 2011-08-21 21:48

>>84-86
I don't code for the C virtual machine as defined in the standard, I code for real architectures. You know what that is? I bet you don't. I bet you're going to explode in a fabulous autistic homosexual rage when I tell you that C is portable assembly. But enough of this, off you go now, your skills in pure C are needed at cash register 6.

Name: Anonymous 2011-08-21 21:52

>>86
So I can't link with functions I have written in other C files? Is that really outside the standard?

Name: i_have_a_raging_boner !!kCq+A64Losi56ze 2011-08-21 21:54

>>87
I don't code for the C virtual machine as defined in the standard

I think you mean the C abstract machine their chief.

I code for real architectures

Programming the lego mindset robot doesn't count as a *real architecture*.

You know what that is? I bet you don't
OMFG, look at that code that does a simple file lock! There are over 5 include guards! And each include guard has a different way to handle the lock for each type of *nix variant!

Name: Anonymous 2011-08-21 22:01

>>89
*I think you mean the C abstract machine their chief*.

Name: Anonymous 2011-08-21 22:07

>>1
Improvements:
OMG ENTERPRISE
Insert and add should return a ``correct or incorrect operation'' exit value, as new does.
Add an interface for getting elements by number of element. If you don't do this you are going to BREAK ENCAPSULATION and this is considered bad practice and forces the programmer, i.e. you to do things like

iter = phone_nums->front;
    while (iter) {
        printf("%s\n", (char*)iter->data);
        if (!strcmp("567-8901", (char*)iter->data)) { temp = iter; }
        iter = iter->next;
    }


The list is fine, It should work correctly.

Name: Anonymous 2011-08-21 22:07

All of you, go back to reddit, I'm begging you.

Name: Anonymous 2011-08-21 22:08

>>91
Or implement an iterator, it shouldn't be difficult.

Name: Anonymous 2011-08-21 22:08

>>92
It is you that comes from ``reddit'', ``faggot''.

Name: Anonymous 2011-08-21 23:14

>>91
Ok, thanks.  How are these?

// these go in llist.c
LLNode* llist_top(LList *self) {
        assert(self != NULL);
        return self->front;
}


LLNode *llist_next(LLNode *node) {
        assert(node != NULL);
        return node->next;
}


So now I can do that kind of thing without breaking encapsulation:
        LLNode *cur_item = llist_top(phone_nums);
        while (cur_item != NULL) {
                printf("%s\n", (char*)cur_item->data);
                cur_item = llist_next(cur_item);
        }


Anything wrong with it now?

Name: Anonymous 2011-08-21 23:24

>>89
Alright fucktard, go play in the traffic. Now.

Name: Anonymous 2011-08-22 1:16

>>95

Assertions are for debugging, not production.  They should be compiled out with -DNDEBUG=1.  You need to actually test your values.

Name: Anonymous 2011-08-22 2:34

>>97
Ok.  I guess I will just
if (self == NULL) {
    exit(1);
}

then.

Name: Anonymous 2011-08-22 2:38

>C virtual machine
>Inline assembler in every library
I lol'd

Name: Anonymous 2011-08-22 2:56

>>98
Yes, it's awesome when programs just abort with no messages and arbitrary exit codes in the middle of running.

    if(self == NULL)
    {
        perror("malloc");    /* in stdio.h */
        exit(1);
    }

You could also #include <errno.h> and exit with that code (but note that the perror call may modify the value, so you'd have to store it in a temp first.)

Name: Anonymous 2011-08-22 3:51

Can all you fucking idiots take this shit back to Reddit.

Name: Anonymous 2011-08-22 4:30

Yeah, this thread is too off-topic!  /prog/ is for talking about bonerlang, PAHKEEMANZ and JEWS!

Name: Anonymous 2011-08-22 5:20

>>102
The thread may be somewhat /prog/, but it really is reddit-quality.

Name: Anonymous 2011-08-22 7:38

I would recommend to stuff the [url=http://paste.pocoo.org/show/462370/]pointers to nodes in the elements of the list itself[/url], as in the Linux kernel (the FreeBSD, which is using macros is also interesting but probably less optimized).

Name: Anonymous 2011-08-22 7:45

>>104
Shit, I suck at BBcode: http://paste.pocoo.org/show/462374/

Name: Anonymous 2011-08-22 8:38

>>105
offsetof is not standard C.

Name: Anonymous 2011-08-22 10:02

>>106

Yes it is.

Name: i_have_a_raging_boner !!kCq+A64Losi56ze 2011-08-22 12:09

>>106
Yes it is you minimum wage bitch. The first mention of 'offsetof' can be found under item 18, section 6.7.2.1 Stucture and union specifiers in the ISO/IEC 9899 standard.

Now shut up and go serve another customer you fucking moron.

Name: Anonymous 2011-08-22 13:14

>>108
minimum wage bitch
Yawn.  You have been saying this every other post for months.  Think of some new shit YOU BROKEN-RECORD BITCH.

Name: i_have_a_raging_boner !!kCq+A64Losi56ze 2011-08-22 13:33

>>108
And you've been making the same ignorant/uneducated statements for the past few years. Either get with the program or take up something that is a tad bit easier. Like working a cash register at Target.

Name: Anonymous 2011-08-22 15:02

It's funny that the absolute worst threads on /prog/ are the ones where someone wants to talk about programming.

Name: n3n7i 2011-08-22 21:45

...Get with the /prog ? ^^

/prog has none =)

Newer Posts
Don't change these.
Name: Email:
Entire Thread Thread List