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\$
  • \$\begingroup\$ It seems the function should just return node_ptr->next. Why you need the loop in the function? \$\endgroup\$ – Mark Shevchenko Jan 18 '15 at 14:27
  • \$\begingroup\$ @MarkShevchenko loop is there to check whether the nod_ptr is in list pointed by first_ptr. \$\endgroup\$ – OldSchool Jan 18 '15 at 16:02
  • 1
    \$\begingroup\$ I think, it's clearer to have the function is_node_in_list here. \$\endgroup\$ – Mark Shevchenko Jan 18 '15 at 16:19
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, privacy policy and cookie policy

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