Skip to main content
Code Review

Return to Answer

deleted 232 characters in body
Source Link
Deduplicator
Deduplicator
  • 19.9k
  • 1
  • 32
  • 65
static linked_list_node_t* merge(linked_list_node_t* a, linked_list_node_t* b) {
    linked_list_node_t* head = NULL;
    linked_list_node_t** insert = &head;

    while (a && b) {
        iflinked_list_node_t** (a->valuechosen <= b->value) {
            *insert = a;
            a =< a->next;
      >value ? }&b else: {&a;
            *insert = b;*chosen;
            b =*chosen= b(*chosen)->next;
        }
        insert = &(*insert)->next
    }

    *insert = a ? a : b;
    return head;
}

Integral remainder is a verysomewhat costly operation. And as you know that 0 <= n < 2 * m in n % m, you can replace it with a cheaper conditional subtraction.

static linked_list_node_t* merge(linked_list_node_t* a, linked_list_node_t* b) {
    linked_list_node_t* head = NULL;
    linked_list_node_t** insert = &head;

    while (a && b) {
        if (a->value < b->value) {
            *insert = a;
            a = a->next;
        } else {
            *insert = b;
            b = b->next;
        }
        insert = &(*insert)->next
    }

    *insert = a ? a : b;
    return head;
}

Integral remainder is a very costly operation. And as you know that 0 <= n < 2 * m in n % m, you can replace it with a cheaper conditional subtraction.

static linked_list_node_t* merge(linked_list_node_t* a, linked_list_node_t* b) {
    linked_list_node_t* head = NULL;
    linked_list_node_t** insert = &head;

    while (a && b) {
        linked_list_node_t** chosen = b->value < a->value ? &b : &a;
        *insert = *chosen;
        *chosen= (*chosen)->next;
        insert = &(*insert)->next
    }

    *insert = a ? a : b;
    return head;
}

Integral remainder is a somewhat costly operation. And as you know that 0 <= n < 2 * m in n % m, you can replace it with a cheaper conditional subtraction.

deleted 12 characters in body
Source Link
Deduplicator
Deduplicator
  • 19.9k
  • 1
  • 32
  • 65
int linked_list_is_sorted(const linked_list_t* list) {
    int last = INT_MIN;
    for(linked_list_node_t* node = list->head; node; node = node->next) {
        if(node->value < last)
            return 0;
        else
            last = node->value;
    }
    return 1;
}

void linked_list_display(const linked_list_t* list) {
    const char* format = "%d";
    for(linked_list_node_t* node = list->head; node; node = node->next) {
        printf(format, node->value);
        separatorformat = ", %d";
    }
}

int linked_list_is_sorted(const linked_list_t* list) {
    int last = INT_MIN;
    for(linked_list_node_t* node = list->head; node; node = node->next)
        if(node->value < last)
            return 0;
        else
            last = node->value;
    return 1;
}

void linked_list_display(const linked_list_t* list) {
    const char* format = "%d";
    for(linked_list_node_t* node = list->head; node; node = node->next) {
        printf(format, node->value);
        separator = ", %d";
    }
}

int linked_list_is_sorted(const linked_list_t* list) {
    int last = INT_MIN;
    for(linked_list_node_t* node = list->head; node; node = node->next) {
        if(node->value < last)
            return 0;
        last = node->value;
    }
    return 1;
}

void linked_list_display(const linked_list_t* list) {
    const char* format = "%d";
    for(linked_list_node_t* node = list->head; node; node = node->next) {
        printf(format, node->value);
        format = ", %d";
    }
}
Source Link
Deduplicator
Deduplicator
  • 19.9k
  • 1
  • 32
  • 65

malloc can fail. Deal with it.

If you don't modify something passed by pointer, mark it const. Const-correctness, if rigorously followed, is a great help for debugging and understanding, and a right pain otherwise.

Consider taking advantage of the possibility to intersperse variable-declarations in code since C99. It's succh a nice feature even ancient C90 compilers allow it as an extension.
This way you can declare and initialize variables where you need them, keeping their scopes minimal and easier to review.

There is nobody stopping you from modifying function-arguments. Might eliminate some variables that way...

ints are easily copied. And sentinels allow for the elimination of special-cases, the bane of elegance and efficiency.

int linked_list_is_sorted(const linked_list_t* list) {
    int last = INT_MIN;
    for(linked_list_node_t* node = list->head; node; node = node->next)
        if(node->value < last)
            return 0;
        else
            last = node->value;
    return 1;
}

Consider const-qualifying all pointers to string-literals. While in C the type is still char[N], it is immutable.

How about changing the format-string instead of an insert?

void linked_list_display(const linked_list_t* list) {
    const char* format = "%d";
    for(linked_list_node_t* node = list->head; node; node = node->next) {
        printf(format, node->value);
        separator = ", %d";
    }
}

The conditional operator (exp ? true_val : false_val) is superb for choosing between two expressions.

Double-pointers are not scary. And using them allows you to avoid needless duplication.

static linked_list_node_t* merge(linked_list_node_t* a, linked_list_node_t* b) {
    linked_list_node_t* head = NULL;
    linked_list_node_t** insert = &head;

    while (a && b) {
        if (a->value < b->value) {
            *insert = a;
            a = a->next;
        } else {
            *insert = b;
            b = b->next;
        }
        insert = &(*insert)->next
    }

    *insert = a ? a : b;
    return head;
}

Integral remainder is a very costly operation. And as you know that 0 <= n < 2 * m in n % m, you can replace it with a cheaper conditional subtraction.

return 0; is implicit for main() since C99. Might be interesting...