views:

317

answers:

6

hi, I am working on some legacy code that defines a linked list (not using STL container)

I want to convert this code so as to use STL list. As you can see in the following example, the linked list is assigned an initial value for all Ns. Then certain elements in the list are assigned some value. Then the "empty elements" in the list are "cleaned up".

I am looking for a better way to do this using STL. especially, can this deleting empty elements code be avoided? I checked STL documentation.. it defines a remove method but thats not exactly what I need here. Is there a way to dynamically allocate linked list size? I would appreciate your suggestions!

Update I have modified the code. this resembles the main code I have. But to avoid any confusion, I am writing a pseudo code below to explain how it works.

Steps

  1. Allocate a size elementIds to the linked list (struct my_list)
  2. There is another linked list meshElem and I am interested in some values from meshElem->elem struct.
    • For Example : I need elemId = meshElem->elem->id; This elemId is in range 0 to elementIds.
    • The elemId will be used as an index to look for a particular element in struct my_list lst e.g. lst[elemId]
  3. In the doSomething () function, loop through 0 to elementIds . In this loop, if certain conditions are satisfied, the lst->number is assigned an integer value =someArray[i] where i is in range 0 to N (done in appendElement)
  4. the elements without next entry in struct my_list lst ,are cleaned up ( Question: can this be avoided ? )
  5. lst->number value is used further in the code for some other processing.

Now the modified code:

struct my_list
 {
  int  number;
  struct my_list *prev;
  struct my_list *next;
 }

void doSomething(void){
     const int N = 50000;
     const int elementIds = 10000;
     int i, elemId, node_i;     
     struct my_list *lst;     
     lst = new struct my_list[elementIds];
     int someArray[12];

     meshElem = mesh->next;     

     for(i=0; i<=elementIds; i++) {
        lst[i].num = 0;
        lst[i].next   = NIL;
        lst[i].prev  = NIL;
      }      

    while(meshElem != NIL){
        // Element id (int) 
        // Note that any elemId will be in range [0 - elemId ]
        elemId = meshElem->elem->id;

        // Do some operations to populate "lst"    
        // Note that 'lst' gets values ONLY for certain 
        // values of i
        for (i = 0; i<=N; i++){        
            // if certain conditions are satisfied, 
            // it updates the  linked list element 
            // lst[meshIdx]. foo1(), foo2() are just some conditions...

            if (foo1()){
               appendElement(someArray[i], &lst[meshIdx])
               }
            else if (foo2()){
               appendElement(someArray[i], &lst[meshIdx])           
            }
        }          

      meshElem = meshelem->next;

  } // End of while(meshElem != NIL)

  // Clean up the linked list lst 
  // by removing unassigned items. 

   struct my_list *lst_2

   for(i=1; i<=N; i++) {
     lst_2 = &lst[i];
     while( lst != NIL ) {      
      if( lst->next != NIL && lst->next->number == 0 ) {
        delete lst_2->next;
        lst_2->next = NIL;
      } // end of if loop
      lst = lst_2->next;

    } // end of while while( lst != NIL )

  } // End of for(i=1; i<=N; i++)


  //  Do some more stuff that uses struct my_list lst
  for(i=1;i<=elementIds;i++) {

      while( lst[i] != NIL && (node_i = lst[i]->number) ) {
            if( node_i == 0) {
              lst[i] = lst[i]->next; 
              continue;
            }
            // Use this "node_i" index in some other arrays to
            // do more stuff. 
            //..
            //..
            //..
            lst[i] = lst[i]->next;    
      }

}


void appendElement(int n, struct my_list *lst) {
  int exists = 0;
  while( lst->next != NIL ) {
    if( lst->number == n ) {
    exists = 1;
    lst=lst->next;
  }
  if( exists < 1 ) {
    lst->number = n2;
    insertElemAfter(lst, 0);
  }
}
A: 
struct my_list {
    int number;
}

void doSomething(void){
    std::list<my_list> lst;
    const int N = 10000;        
    for (int i = 0; i<=N; ++i) {        
        if (1 == foo(N)) { //I'm guessing this is what you meant
            my_list new_element; //Initalize with something;
            lst.push_back(new_element);
        }
    }
}

int
foo(int n) {
    // some function that returns 0 or 1 based on 
    // the value of n
    return 1;
}
Andreas Brinck
@dribeas Thansk, feel free to edit yourself next time ;)
Andreas Brinck
A: 
std::list<int> lst;
Will
A: 

In your logic instead of creating all the nodes in the beginning why dont you run the loop only once and create one elements dynamically for true condition and add it in the list.

for (i = 0; i<=N; i++)
{
    if ( i == foo(N) )
        node = createNode();
    appendElement(node);
}
GJ
What's up with braces that everybody is getting them wrong today?
David Rodríguez - dribeas
Finally, people are using braces correctly!
John Dibling
A: 

I think you just want:

std::list<int> lst;

for (i = 0; i<=N; i++){        
    if ( i == foo(N) )
    lst.push_back(/*some new value*/);
}  
Autopulated
+1  A: 

A linked list typically do not use contiguous memory, but rather fragmented heap-allocated nodes. I guess if you provide the std::list constructor with an initial count, at least that many nodes will be contiguous. Other than that, you'd need to write your own allocator to go with std::list.

Johann Gerell
No, std::list makes no such guarantee for the constructor that takes a count.
anon
+6  A: 

Your legacy linked list is essentially a threaded sparse vector. The array with NULLs is the sparse vector, and the linked list provides the threading. The two combined give constant access to individual nodes (random access into the array) and efficient traversal over "valid" nodes (avoiding NULLs).

Assuming both of these aspects are important, and assuming the Data maybe more complex than the simple int you show, then you could create a data structure such as:

class ThreadedSparseVector {
private:
    std::vector<Data*> m_thread;
    std::vector<int> m_sparse_vector;
};

During initialization, you can pre-size m_sparse_vector and initialize the values to -1. As you append elements (or access them), check if it is already "valid" first, adding it to the thread if not:

void ThreadedSparseVector::appendElement(int i) {
    if (-1 == m_sparse_vector[i]) {
        // Add new data
        m_sparse_vector[i] = m_thread.size()
        m_thread.push_back(new Data(i));
    }

    Data* data = m_thread[m_sparse_vector[i]];
    // Initialize/update data as necessary
}

If the threading is more important than random access, another option is to simply use an STL map. If random access is more important than threading, then you can simply use an STL vector and tolerate NULLs during iteration (or create a custom iterator that automatically skips NULLs).

Another alternative, depending on your motivation to convert to STL, is to create a wrapper around the legacy code that implements an STL-compatible interface, as opposed to converting the data structure itself to use STL.

Jason Govig
great detailed answer
hi Jason, Thank you for a detailed answer! I have updated my question ( before looking at your answer) . The Data is <int> in this case (please see my updated question) . so I think your analysis still holds good. I will give it a try.
cppb