From 3f37fdbff489bfe5ba7c46b36cf5ece920fdd783 Mon Sep 17 00:00:00 2001
From: Michael Tryby <tryby.michael@epa.gov>
Date: Thu, 11 Apr 2019 10:45:22 -0400
Subject: [PATCH] Refactoring head_list and tail_list

Simplifying head and tail list. Adding delete_node() to list API.
---
 src/util/list.c          | 48 +++++++++++++---------------------------
 src/util/list.h          | 10 +++++++--
 tests/util/test_list.cpp | 22 +++++++++---------
 3 files changed, 34 insertions(+), 46 deletions(-)

diff --git a/src/util/list.c b/src/util/list.c
index d7519b8..d3e41d8 100644
--- a/src/util/list.c
+++ b/src/util/list.c
@@ -62,12 +62,7 @@ void delete_list(list_t *list)
     while(list->head != NULL) {
         current = list->head;
         list->head = current->next;
-
-        if (list->freeFn)
-            list->freeFn(current->data);
-
-        free(current->data);
-        free(current);
+        delete_node(list, current);
     }
     free(list);
 }
@@ -119,47 +114,26 @@ void for_each_list(list_t *list, listIterator iterator)
     }
 }
 
-void *head_list(list_t *list, bool removeFromList)
+list_node_t *head_list(list_t *list, bool removeFromList)
 //
-// Warning: Caller is responsible for freeing the node->data returned.
+// Warning: When node is removed caller is responsible for freeing it.
 //
 {
     assert(list->head != NULL);
 
     list_node_t *node = list->head;
-    // Allocating and copying pointer to node data
-    void *element = (void *)malloc(list->elementSize);
-    memcpy(element, node->data, list->elementSize);
-
     if(removeFromList) {
         // Disconnecting head node
         list->head = node->next;
         list->logicalLength--;
-
-        // Freeing pointer to node->data and node
-        free(node->data);
-        free(node);
     }
-    // Now element points to data formerly pointed to by node. Caller
-    // is responsible for freeing both pointer to data and data.
-    return element;
+    return node;
 }
 
-void *tail_list(list_t *list)
-//
-// Warning: Caller is responsible for freeing the node->data returned.
-//
+list_node_t *tail_list(list_t *list)
 {
     assert(list->tail != NULL);
-
-    list_node_t *node = list->tail;
-    // Allocating and copying pointer to node data
-    void *element = (void *)malloc(list->elementSize);
-    memcpy(element, node->data, list->elementSize);
-
-    // Pointer to element data gets returned. Caller is responsible
-    // for freeing pointer to data.
-    return element;
+    return list->tail;
 }
 
 int size_list(list_t *list)
@@ -172,9 +146,17 @@ void *get_data(list_node_t *lnode)
     return lnode->data;
 }
 
+void delete_node(list_t *list, list_node_t *lnode)
+{
+    if (list->freeFn)
+        list->freeFn(lnode->data);
+
+    free(lnode->data);
+    free(lnode);
+}
 
 //
-// Iterator first/done/next operations
+// Iterator first/done/next operations provide containment for list abstraction
 // http://www.cs.yale.edu/homes/aspnes/pinewiki/C(2f)Iterators.html
 // Accessed on April 11, 2019
 //
diff --git a/src/util/list.h b/src/util/list.h
index fe6ffc7..090fa4a 100644
--- a/src/util/list.h
+++ b/src/util/list.h
@@ -59,11 +59,17 @@ void append_list(list_t *list, void *element);
 */
 int size_list(list_t *list);
 
+
 /**
 @brief Returns pointer to list node's data.
 */
 void *get_data(list_node_t *lnode);
 
+/**
+@brief Frees memory associated with a list node.
+*/
+void delete_node(list_t *list,  list_node_t *lnode);
+
 
 /**
 @brief Calls the supplied iterator function with the data element of each
@@ -74,12 +80,12 @@ void for_each_list(list_t *list, listIterator iterator);
 /**
 @brief Returns the head of the list (optionally removing it at the same time).
 */
-void *head_list(list_t *list, bool removeFromList);
+list_node_t *head_list(list_t *list, bool removeFromList);
 
 /**
 @brief Returns the tail of the list.
 */
-void *tail_list(list_t *list);
+list_node_t *tail_list(list_t *list);
 
 
 /**
diff --git a/tests/util/test_list.cpp b/tests/util/test_list.cpp
index aee21fc..df02886 100644
--- a/tests/util/test_list.cpp
+++ b/tests/util/test_list.cpp
@@ -116,25 +116,21 @@ BOOST_FIXTURE_TEST_CASE(test_string_list, FixtureStrings) {
 
 BOOST_FIXTURE_TEST_CASE(test_head_list, FixtureStrings) {
 
-    void *temp = head_list(list, true);
+    BOOST_CHECK(check_string(get_string_data(head_list(list, false)), "David"));
+    BOOST_CHECK(size_list(list) == 5);
 
-    BOOST_CHECK(check_string(*(char **)temp, "David"));
+    list_node_t *lnode = head_list(list, true);
+    BOOST_CHECK(check_string(get_string_data(lnode), "David"));
     BOOST_CHECK(size_list(list) == 4);
 
-	// To free a node, free both the data and reference to data
-    free_string(temp);
-    free(temp);
+    delete_node(list, lnode);
 }
 
 
 BOOST_FIXTURE_TEST_CASE(test_tail_list, FixtureStrings) {
 
-    void *temp = tail_list(list);
-
-    BOOST_CHECK(check_string(*(char **)temp, "Jimi"));
+    BOOST_CHECK(check_string(get_string_data(tail_list(list)), "Jimi"));
     BOOST_CHECK(size_list(list) == 5);
-
-    free(temp);
 }
 
 
@@ -198,7 +194,8 @@ BOOST_AUTO_TEST_CASE(test_struct_list){
 
     BOOST_CHECK(size_list(list) == 3);
 
-    //for_each_list(list, iterate_test_data);
+    for_each_list(list, iterate_test_data);
+
 
     list_node_t *lnode;
     // Iterate over list while maintaining containment of abstraction
@@ -207,6 +204,9 @@ BOOST_AUTO_TEST_CASE(test_struct_list){
         printf("Found number: %i name: %s\n", test_data->num, test_data->name);
     }
 
+    lnode = head_list(list, true);
+    delete_node(list, lnode);
+
     delete_list(list);
 }