current community

your communities

more stack exchange communities

Stack Exchange
tour help
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.
up vote 2 down vote favorite

I am very new to C but have experience with languages such as Java and Python. I have a small code segment where I am trying to implement a stack pop operation. My question is how do I indicate that the pop failed because the linked list was empty? Should I put some special "error" value in the integer variable where the popped values is supposed to go? I'd appreciate if you can find any other flaws in the code or a way to improve it. Thank you?

sll * pop(sll *head, int *rt) {
   if(head == NULL) {
       // *rt = NULL;
       return NULL;
   }
   *rt = head->value;
   sll * curr = head->next;
   free(head);
   return curr;
}
share|improve this question
1  
This is a question more appropriate for Stack Overflow. –  Jeff Mercado Dec 1 '11 at 22:24
    
There isn't consensus on whether or not these sorts of questions belong here, if you have an opionion share it on the meta question: meta.codereview.stackexchange.com/questions/383/… –  Winston Ewert Dec 2 '11 at 14:57
    
Keep in mind that it is perfectly fine to pass a NULL pointer to free(), nothing will happen if you do so. –  Lundin Dec 9 '11 at 13:28

2 Answers 2

up vote 3 down vote accepted

Change the method as follows:

bool pop( sll** head, int* rt)

(bool assumes C99 or later; if your compiler does not support it, use int instead.)

This way, you receive a pointer to a pointer to the head, so you can modify the head as you like, and you return true or false to indicate success or failure.

EDIT: So, your method will now look like this: (WARNING: I have not tested it!)

bool pop( sll** head, int* rt )
{
    if( *head == NULL )
        return false;
    *rt = (*head)->value;
    sll* curr = (*head)->next;
    free( *head );
    *head = curr;
    return true;
}
share|improve this answer
    
Or alternatively return a typedef:ed enum corresponding to a specific error code, if you want more advanced error handling than "it didn't work". –  Lundin Dec 9 '11 at 13:25
up vote 1 down vote

Since you are returning a pointer returning NULL in case of an error is a perfectly fine way of handling an error condition. A lot of C libraries handle it the exact same way.

share|improve this answer
3  
-1: Yes BUT you can't tell the difference between an error and correctly extracting the last element. So if you are testing weather to use the value in *rt you cant use NULL as your test (as removing the last element results in a NULL return and set rt correctly, while a failure returns NULL and does not change rt). –  Loki Astari Dec 1 '11 at 23:09

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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