3

I am given a sparse-array called "head", which is two-dimensional having an index, and a value. So something like : (3, 100) (6,200) (8,100)

  1. I have to insert a node (value, index) into this sparse array in ascending order. So if I am given (2,100), the list should look like: (2, 100) (3,100) (6,200) (8,100)

Similarly if I am given (4,200), it should return (3,100) (4,200) (6,200) (8,100)

Condition 1: if the indices are same, then I must add the values

So if I am given (3,100), then I should return (3,200) (6,200) (8,100)

Condition 2: if the indices are same, and the value is zero, then the value should be removed. So if the array is (3,-100), I must return

(6,200) (8,100)

Node * List_insert_ascend(Node * head, int value, int index)
{
  Node * node = List_create(value, index); //this creates an empty node, "node"

  if (index < (head->index)) //node's index is less, e.g. (1,100)
    {node -> next = head;} //this inserts "node" before "head"
  if (index == (head->index))
  {
    node = head;
    head->value = head->value + value; //Condition 1
    while ((head->value)==0)  //Condition 2
    {
      Node *p = head->next;
      head = p;
        
    }
  }
  return node;

}

My understanding is that when I make head->next the new head, that should get rid of the original entry.

But the 0 value-ed index continues to remain in the list. Result is (3,0) (6,200) (8,100)

If someone can help me with figuring out what I'm doing wrong (and perhaps even why) I would really appreciate it.

Community
  • 1
  • 1
  • In List_insert_ascend method, you are given the pointer to head node of the list. So as the first step, you actually have to loop through the nodes in the list to see which node has a matching index. If no node has a matching index, create a new node. If a specific node does have the same index, add the value of that node with the given value in List_insert_ascend. Now check if the resulting value is 0. If it is, then delete the node. at the end of the entire method, you should return the head of the resulting list, not an arbitrary node. – sufinawaz Oct 22 '13 at 19:22

2 Answers2

2

You have undefined behavior in your code.

When you do

Node *p = head->next;
head = p;
free(p);

you're actually freeing the node that both head and p points to. Then dereferencing head leads to undefined behavior.

But that's not the only problem. The other is that you don't actually unlink the node you're freeing. The previous head->next (from before the reassignment of head and its subsequent freeing) pointer still points to the now free node.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621
  • Thanks, I just got rid of the free(p) line. But I'm still failing to see where the problem is. – wickedcolor Oct 22 '13 at 18:42
  • @user2826609 After the edit, you don't actually remove any node, just loop as long as `head->value == 0`. If `head->value` is not zero, then you just exit the loop and return the head of the list. – Some programmer dude Oct 23 '13 at 06:55
1

your function should return the new head either by return head or by Node **head as parameter

and

head->index is a crash iff you have no head at all

Node * list_insert_update_remove(Node **head, int value, int index) 
{
  Node *node = List_create(...);
  if (*head == NULL) 
    *head = node;
  else {
    Node *prev = NULL;
    Node *list = head;
    while (list) {
      if (index < list->index) { //prepend
        if (prev == NULL) // before head
          *head = node; 
        else {
          prev->next = node; // into the middle/end
          node->next = list;
        }
        break;
      } else if (index == list->index) {
        //update or remove (execercise)
        break;
      } else if (list->next == NULL) { // append at end
        list->next = node;
        break;
      }
      prev = list;
      list = list->next;
    }
  }

  return *head;
}
Peter Miehle
  • 5,984
  • 2
  • 38
  • 55