2
\$\begingroup\$

What is considered to be a good programming practice for returning value from the function? For example, if we have a function in C named list* after( list *node), which returns the node which is after the node pointer.

First option:

node *after(node *first_ptr, node *nod_ptr)
{

    if (!(is_last(first_ptr, nod_ptr)) && !( first_ptr==NULL) && nod_ptr != NULL ) {

        while (first_ptr != nod_ptr && first_ptr != NULL){

            first_ptr = first_ptr->next;

        }

        return first_ptr != NULL ? first_ptr->next : NULL;

    }

     return NULL;

} 

Second option:

node *after(node *first_ptr, node *nod_ptr)
{

    node *temp=NULL;
    if (!(is_last(first_ptr, nod_ptr)) && !( first_ptr==NULL) && nod_ptr != NULL ) {

        while (first_ptr != nod_ptr){

            first_ptr = first_ptr->next;
            temp = first_ptr;
            if(temp == NULL){

              break;
            }
            temp = temp->next;
        }


    }

     return temp;

} 

Here node is typedef structure and next is a pointer to next element in the list and is_last(node *,node *) checks whether the second argument pointer is the last element in the list having start pointer as the first argument.

I'm wondering, in general, if big functions are necessary. Which seems nicer to understand?

\$\endgroup\$
3
  • \$\begingroup\$ It seems the function should just return node_ptr->next. Why you need the loop in the function? \$\endgroup\$ Commented Jan 18, 2015 at 14:27
  • \$\begingroup\$ @MarkShevchenko loop is there to check whether the nod_ptr is in list pointed by first_ptr. \$\endgroup\$
    – OldSchool
    Commented Jan 18, 2015 at 16:02
  • 1
    \$\begingroup\$ I think, it's clearer to have the function is_node_in_list here. \$\endgroup\$ Commented Jan 18, 2015 at 16:19

1 Answer 1

2
\$\begingroup\$

Your after() function is actually doing two things: it returns nod_ptr->next, but with the stipulation that the list starting at first_ptr contains nod_ptr. Therefore, you should decompose it into two functions.

/**
 * Returns the content node if it is contained in the list starting at
 * head, otherwise NULL.  The list must not contain a cycle.
 */
node *find(node *head, node *content)
{
    for (node *n = head; n != NULL; n = n->next)
    {
        if (n == content)
        {
            return content;
        }
    }
    return NULL;
}

/**
 * If the list starting at head contains content, returns content->next.
 * Otherwise, returns NULL.
 */
node *after(node *head, node *content)
{
    return (content != NULL && find(head, content)) ? content->next
                                                    : NULL;
}
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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