complex struct access question.

[CaptainBly]

Okay I've been spending lots of time on this and it's giving me a migraine so I go humble and ask the guru's here.

I have several structs

typedef struct vert
{
int x,y,z;
struct vert * next;
struct vert * prev;
} vertex_type;
 
typedef struct poly
{
int x,y,z;
struct poly * next;
struct poly * prev;
} polygon_type;
 
//and finally.
typedef struct object
{
char name=[20];
struct vert *vertex;
struct poly *polygon;
 
struct object * next;
struct object * prev;
} obj_type
 
 

And I initilize as such.

 
obj_type object1={NULL};
obj_type *firstobj;
 
 

Once my member functions are initilized and filled and the vertex pointers are filled and linked (which at this point I think is happening *fingers crossed* but am probably wrong given my problem below. )

I try and retrieve some data via, some function which takes the first object as a pointer and winds the polygon pointer to the end of the polygon list.

func(obj_type *current)
{
polygon_type *tmp;
while((current->polygon->next) != NULL)
    {
                           tmp=current->polygon;
                           current->polygon=tmp->next;
}
 
}
 

Why does it crash with a segmentation fault when it first tries to process the while statement? Am I accessing the variable incorrectly? I have tried using

current->polygon.next
 

but my compiler says that is not defined.


Or maybe my error is in the code for creating new objects vertex's or polygons, or perhaps in the code that free them at the end.

 
#include <stdlib.h>
#include <stdio.h>
#include "main.h"
 
int addobj(obj_type *last) {
 
    obj_type *tmp=NULL;
    while(last->next!=NULL)
    {
                           tmp=last;
    last=tmp->next;
}
    tmp=(obj_type*) malloc(sizeof(obj_type));
    tmp->prev = last;
    tmp->next = NULL;
    last->next = tmp;
    last=tmp;
    printf("Next obj node Created.%r");
    return 1;
  }
 
  int freeobjlist(obj_type *current) {
    obj_type *tmp;
    while(current->next!=NULL)
    {
                           tmp=current;
                           current=tmp->next;
 
                           }
    while(current->prev!=NULL)
    {
 
                           freepolygons(current);
                           freeverts(current);
                           freetextcoords(current);
 
                           tmp=current->prev;
                           free(current);
                           current=tmp;
    printf("obj removed.%r");
                           }
 
 
 
 
 
  return 1;
 
  }
 
  int addvertex(obj_type *current)
  {
 
          vertex_type *tmp;
    if(current->vertex==NULL)
    {         
              tmp=(vertex_type*)malloc(sizeof(vertex_type));
                           current->vertex=tmp;
                           current->vertex->prev=NULL;
                           current->vertex->next=NULL;
                           printf("First vertext created.%r");
                           return 1;
                           }
    else
    {
      while(current->vertex->next!=NULL)
      {
                             tmp=current->vertex->next;
                             current->vertex=tmp;
                       }
      tmp=(vertex_type*)malloc(sizeof(vertex_type));
      current->vertex->next=tmp;
      tmp->prev=current->vertex;
      tmp->next=NULL;
      current->vertex=tmp;
      printf("Additional Vertex added.");
      return 1;
 
 
 
 
        }
 
  return 0;
 
      }
 
 
 
int addpolygon(obj_type *current)
  {
          polygon_type *tmp;
    if(current->polygon==NULL)
    {         
              tmp=(polygon_type*)malloc(sizeof(polygon_type));
                           current->polygon=tmp;
                           current->polygon->prev=NULL;
                           current->polygon->next=NULL;
                           printf("First polygon created.%r");
                           return 1;
                           }
    else
    {
      while(current->polygon->next!=NULL)
      {
                             tmp=current->polygon->next;
                             current->polygon=tmp;
                       }
      tmp=(polygon_type*)malloc(sizeof(polygon_type));
      current->polygon->next=tmp;
      tmp->prev=current->polygon;
      tmp->next=NULL;
      current->polygon=tmp;
      printf("Additional Vertex added.");
      return 1;
 
 
 
 
        }
 
  return 0;
 
      }
 
  int addtextcoord(obj_type *current)
  {
          textcoord_type *tmp;
    if(current->textcoord==NULL)
    {         
              tmp=(textcoord_type*)malloc(sizeof(textcoord_type));
                           current->textcoord=tmp;
                           current->textcoord->prev=NULL;
                           current->textcoord->next=NULL;
                           printf("First textcoord created.%r");
                           return 1;
                           }
    else
    {
      while(current->textcoord->next!=NULL)
      {
                             tmp=current->textcoord->next;
                             current->textcoord=tmp;
                       }
      tmp=(textcoord_type*)malloc(sizeof(textcoord_type));
      current->textcoord->next=tmp;
      tmp->prev=current->textcoord;
      tmp->next=NULL;
      current->textcoord=tmp;
      printf("Additional textcoord added.");
      return 1;
 
 
 
 
        }
 
  return 0;
 
      }
 
int freeverts(obj_type *current) {
    vertex_type *tmp;
    while(current->vertex->next!=NULL)
    {
                           tmp=current->vertex;
                           current->vertex=tmp->next;
 
                           }
    while(current->vertex->prev!=NULL)
    {
                           tmp=current->vertex->prev;
                           free(current->vertex);
                           current->vertex=tmp;
    printf("vertex removed.%r");
                           }
    if(current->vertex!=NULL)
    {
              free(current->vertex);
              current->vertex=NULL;
                          }
    printf("Master vertex cleared.%r");
 
 
 
 
  return 1;
 
  }      
 
int freepolygons(obj_type *current) {
    polygon_type *tmp;
 
 
    while((current->polygon->next) != NULL)
    {
                           tmp=current->polygon;
                           current->polygon=tmp->next;
                           printf("polygon wound to end.");
                           fflush(stdout);
                           }
    while(current->polygon->prev!=NULL)
    {
                           printf("Polygon deletion initiated.");
                           fflush(stdout);
                           tmp=current->polygon->prev;
                           free(current->polygon);
                           current->polygon=tmp;
    printf("polygon removed.%r");
                           }
    if(current->polygon!=NULL)
    {
              free(current->polygon);
              current->polygon=NULL;
                          }
    printf("Master polygon cleared.%r");
 
 
 
 
  return 1;
 
  }
 
int freetextcoords(obj_type *current) {
    textcoord_type *tmp;
    while(current->textcoord->next!=NULL)
    {
                           tmp=current->textcoord;
                           current->textcoord=tmp->next;
 
                           }
    while(current->textcoord->prev!=NULL)
    {
                           tmp=current->textcoord->prev;
                           free(current->textcoord);
                           current->textcoord=tmp;
    printf("textcoord removed.%r");
                           }
    if(current->textcoord!=NULL)
    {
              free(current->textcoord);
              current->textcoord=NULL;
                          }
    printf("Master textcoord cleared.%r");
 
 
 
 
  return 1;
 
  }  
 

Any help is appreciated as I am apparently missing something in the way I am attempting access members of my structures and my brain is starting to feel a bit melty.

[weaknessforcats]

This code:

while((current->polygon->next) != NULL)
etc...

will crash if either current or polygon are null or otherwise contain addresses outside your process address space. As a rule, you should check pointers for null before applying the indirection operator.

Other observations are:
1) there is a main.h, which you don't show but which I suspect os loaded with global variables. There cound be problem there.
2) the overall code struture is "stream of consiousness". That is, steps are coded in a do-this-then-do-that order as the steps occurred to you.
3) no cncapsulation. There is next/prev/current pointers scattered all thought the code. Your linked lists are inextricably meshed with the application.

A properly written linked list will have 5 lines of code referring to the next/prev pointers of a node.

If you have a list of vertices or a list of polygons, where are the structs for the lists?? All you have are structs for the nodes.

This code:

while((current->polygon->next) != NULL)
etc...

will crash if either current or polygon are null or otherwise contain addresses outside your process address space. As a rule, you should check pointers for null before applying the indirection operator.

Other observations are:
1) there is a main.h, which you don't show but which I suspect os loaded with global variables. There cound be problem there.
2) the overall code struture is "stream of consiousness". That is, steps are coded in a do-this-then-do-that order as the steps occurred to you.
3) no cncapsulation. There is next/prev/current pointers scattered all thought the code. Your linked lists are inextricably meshed with the application.

A properly written linked list will have 5 lines of code referring to the next/prev pointers of a node.

If you have a list of vertices or a list of polygons, where are the structs for the lists?? All you have are structs for the nodes.

Maybe you should re-desgin your structs:

typedef struct vert
{
  int x,y,z;
} vertex_type;
 
typedef struct poly
{
  int x,y,z;
} polygon_type;
 
//and finally.
typedef struct object
{
char name[20];
LinkedList* vertex;
LinkedList* polygon;
 } obj_type;
 

Here the linked lists have been removed from the data structs. Only the object has pointers to the linked lists
of vertices and polygons.

That means you need structs for the LinkedList and the Nodes in that list:

struct Node
{
     union
     {
         obj_type* thedata;
         vertex_type* vertex;
         polygon_type* polygon;
     };
     Node* next;
     Node* prev;
};
struct LinkedList
{
    Node* start;
    Node* end;
    Node* current;
};
 

Here you see the Node contains only a pointer to the data. Now you can write linked list functions without
paying attention to what's in the list. Here is a possible function to create a linked list:

LinkedList* CreateLinkedList()
{
    LinkedList* temp = new LinkedList;
    temp->start = 0;
    temp->end = 0;
    temp->current = 0;
    return temp;
}
 

You can use this function inside another function that creates your object:

object* CreateObject()
{
       object* temp = new object;
       temp->polygon = CreateLinkedList();
       temp->vertex = CreateLinkedList();
       temp->name[0] = '\0';
       return temp;
}
 

Now in main(), all you do is create an object:

nt main()
{
 
    object*  TheShape = CreateObject();
}
 

From here you write functions with object pointers that add the object name or add polygons or vertexes to the object's
linked lists.

Make some rules:
1) No reference to the LinkedList start, end, current members unless the function has a LinkedList* argument.
2) No reference to the Node next/prev members unless the function has Node* arguments.

Lastly, this style of code:

]
current->textcoord->next=tmp;

is called spaghetti code. It's a bad style since it exposes the member names of several structs.
Should you need to redesign any of the structs, this line will break. This will force a re-code of the entire
application. Further, if either current or textcoord is null, you crash.
Further, if either current or textcoord contains a bad address, you crash.

[CaptainBly]

Thank you for the great reply, though I'm still not sure I understand the flow of what you are doing.

if I were to create a linked list of objects how would I access the member functions?

as such?

LinkedList *objects = createobject();
 
objects->current->vertex->x=1;
 
objects->current->vertex->y=2;
objects->current->vertex->z=3;
 

And then when I add another object later do I just

objects->next=new object;
 

Assuming the function returns a valid memory location for a obj_type.

And to increment the list,

 
objects->current=objects->next;
 
 

And what about adding a vertex to the structure.

would it then be

objects->current->vertex->next=new vertex;
objects->current->vertex->current=next;
 

Is this correct? I feel I can almost see the way your code works out in the end.

[weaknessforcats]

if I were to create a linked list of objects how would I access the member functions?

as such?

Code: ( text )
LinkedList *objects = createobject();

objects->current->vertex->x=1;

objects->current->vertex->y=2;
objects->current->vertex->z=3;

You are still persisting in doing things in a serial manner. Please try to stop doing this.

You create a linked list and add objects to it:

object*  TheShape = CreateObject("ComplexShape");
LinkedList theObjects = CreateLinkedList();
AddtoEnd(theObjects, TheShape);
 

The AddToEnd() would look like:

void AddToEnd(LinkedList* thelist, object* theobject)
{
   Node* temp = CreateNode(theobject);
 
   if (thelist->start== 0)
   {
       //list is empty
       thelist->start = temp;
       thelist->end = temp;
       returnl;
    {
    InsertAfter(thelist->end, temp);
 }
 

Here the object is added to the end of the linked list. The InsertAfter is a Node function and it needs to know what the data in the union is to be.

The CreateNode(), creates a Node on the heap and assigns the data member to the union.

Node* CreateNode(object* data)
{
      Node* temp = new Node;
      temp->thedata = data;
      temp->next = 0;
      temp->prev = 0;
      return temp;
}
 

This InsertAfter(Node* first, Node* second) inserts the second Node after the first. I needs to handle the case where first->next is 0. The case where first->prev is 0. And the case case where neither are zero. The function is not concerned about the data in the Node but only about placing the second argument after the first.

You can use this function to insert a new node to the start of the list by calling it with the new node as the first argument and the start of the list as the secnd argument.

You can use this function to insert a new node to the end of the list by calling it with the new node as the second argument and the end of the list as the first argument.

You can use this function to insert a new node in thie middle of the list by calling it with the new node as the second argument and the Node to insert after as the secnd argument.

The code might be:

void InsertAfter(Node* first, Node* second)
{
        //Case 1: first->next is zero
        if (!first->next)
        {
            //first->prev does not need to change
            first->next = second;
            second->prev = first;
            //second->next does not need to change
            return;
        }
        //first->next is not zero
 
 
        second->prev = first;
        second->next = first->next;
 
        first->next->prev  = second;
        first->next = second;
            //first->prev not changed
 
}
 

If you do this correctly, the above function will be the only function in your application than mentions the next/prev pointers of a Node.

Then this:

if I were to create a linked list of objects how would I access the member functions?

as such?

Code: ( text )
LinkedList *objects = createobject();

objects->current->vertex->x=1;

objects->current->vertex->y=2;
objects->current->vertex->z=3;

Not this way. Remember the user does not know about start/end/current. You need functions here.

object*  TheShape = CreateObject("ComplexShape");
LinkedList theObjects = CreateLinkedList();
AddtoEnd(theObjects, TheShape);
//the object is in the list but the pointer to it is still valid:
 
TheShape->AddVertextToEnd( 3,4,5);       //this is your member function
TheShape->AddPolygonToEnd(5,6,7);
 

So this might be OK:

void object::AddVertexToEnd(int x, int y, int z)
{
     Node* temp = CreateVertexNode(x,y,z);
     AddToEnd(this->vertex, temp);   //from above
}
 

where:

Node* CreateVertextNode(int x, int y, int x)
{     vert* obj = new vert(x,y,x)
      temp->vertex = vert;
      temp->next = 0;
      temp->prev = 0;
      return temp;
}
 

That's enough hints. You'll have me coding the whole thing.

Keep mind when you code a->b->c->d a big flag should appear with giant words: WRONG WAY.

[CaptainBly]

Ok I started a separate program in order to help me more fully understand the way you organize your data. Let me know if I'm getting the gist of it or if you know a good reference I should look into (I.E. Linked Lists for the Dense).

 
#include <cstdlib>
#include <iostream>
 
using namespace std;
 
 
 
 
struct aNode {
       union {
             struct aData *a;
             struct bData *b;
             };
       aNode *next;
       aNode *prev;
};
 
struct aLinkedList {
       aNode *start;
        aNode *end;
        aNode *current;
       };
 
struct aData {
       int x;
        aLinkedList *MybList;
       };
 
struct bData {
       char *y;
       };
 
aLinkedList *newlinkedlist();
aData *Create_aData();
bData *Create_bData();
void connectnodetolist(aLinkedList *,aNode *);
aNode* CreateNode();
void listprint(aLinkedList*);       
int main(int argc, char *argv[])
{
    //create list
    aLinkedList *MyaList=newlinkedlist();
 
    //create node for list;
    aNode *MyaData=CreateNode();
    MyaData->a=Create_aData();
    //connect MyaData to MyaList;
    connectnodetolist(MyaList,MyaData);
    //And again with bData
    aNode *MybData=CreateNode();
    MybData->b=Create_bData();
 
    //but First MybList must be initilized
    MyaList->current->a->MybList=newlinkedlist();
    //connect MybData to MyaList current data member MybList;
    connectnodetolist(MyaList->current->a->MybList,MybData);
 
 
    listprint(MyaList);
 
 
 
    system("PAUSE");
    return EXIT_SUCCESS;
}
 
aLinkedList *newlinkedlist()
{
    aLinkedList *temp=new aLinkedList;
    temp->start=0;
    temp->end=0;
    temp->current=0;
 
    return temp; 
     }
aData *Create_aData()
{
     aData *node=new aData;
     node->x=1;      
     return node;
      }
 
bData *Create_bData()
{
     bData *node=new bData;
     node->y="Thisisit";      
     return node;
      }
 
void connectnodetolist(aLinkedList *mylist,aNode *mydata)
{
     if(mylist->start==0)
     {
                         mylist->start=mydata;
                         mylist->end=mydata;
                         mylist->current=mydata;
                         return;
                         }
 
     mylist->end->next=mydata;
     mydata->prev=mylist->end;
     mylist->end=mydata;
     mylist->current=mydata;
     return;
     }
 
 
aNode* CreateNode()
{
   aNode *node=new aNode;
   node->next=0;
   node->prev=0;
   return node;
     }
void listprint(aLinkedList *theList)
{
//this next bit should probably just be a rewind to start of list function;
if(theList->start==0) return;
if(theList->current==0)
{
theList->current=theList->start;
                     }
aData *aTemp=theList->current->a;
bData *bTemp=aTemp->MybList->current->b;                     
printf("theList->current->a->x = %d \n",aTemp->x);
printf("theList->current->a->MybList->b->y = %s\n",bTemp->y);
 
 
           }     
 
 

[weaknessforcats]

This code is much, much better.

//create list
aLinkedList *MyaList=newlinkedlist();

//create node for list;
aNode *MyaData=CreateNode();
MyaData->a=Create_aData();
//connect MyaData to MyaList;
connectnodetolist(MyaList,MyaData);
//And again with bData
aNode *MybData=CreateNode();
MybData->b=Create_bData();

//but First MybList must be initilized
MyaList->current->a->MybList=newlinkedlist();
//connect MybData to MyaList current data member MybList;
connectnodetolist(MyaList->current->a->MybList,MybData);

Here there is still too much exposed in main(). The user in main() should not need to know about nodes.

You create an object list:

 aLinkedList *MyaList=newlinkedlist();
 

Then you add objects to the list:

aData* obj = CreateObject(245);
AddToList(MyaList, obj);
 

and that's all you see in main(). I made up the vaue for aData. I may have that wrong in concept but an object usually needs a value.

The CreateObject(int value) takes care of creating a new aData and initialize ALL of it'e members:

aData* CreateObject()
{
    aData* temp = new aData;
    temp->MybList = newlinkedlist();
    temp->x = value;
    return temp;
}
 

It looks just like your newlinkedlist() function.

Notice all values are on the heap to avoid the compiler deleting something that goes out of scope. Using the heap here is very important.

Keep going in this direction. When it;s done right:

1) in main() there are only references to linked lists.
2) functions with only linkedlist* arguments are the only places where start, end and current are adjusted.
3) functions with only Node* arguments are the only places where the Node members are adjusted.

It's heartwarming to see someone try this out. Keep posting as necessary.

[CaptainBly]

My code eventually evolved to this.

//linklist.h
 
class Node
{
 
           private:
 
                void *d;
           Node* n;
           Node* p;                    
 
           public:
           void* obj() {return d;}
           Node* next() {return n;}
           Node* prev() {return p;}
 
           void next(Node *w) {n=w;}
           void prev(Node *w) {p=w;}
           Node(void *vp){n=0; p=0; d=vp;}
           Node() {n=0; n=0; d=0;}
      };
struct LinkedList 
{
          private:
          Node* start;
          Node* end;
          Node* current;
          int index;
          public:
          void* element(unsigned short); //set current to node # max node # = qty;
          void* obj() {return current->obj();}
          unsigned short add(void *);
          void rewind() {current=start;}   
          void* next() {current=current->next(); return current->obj();}
          void* prev() {current=current->prev(); return current->obj();}
          int qty() {return index;}
          LinkedList(void *vp) {add(vp);}
          LinkedList() {start=0; end=0; current=0; index=0;}     
      } ;
 
//linkedlists.cpp
 
unsigned short LinkedList::add(void *vPtr)
{
 
            Node *nTemp=new Node(vPtr);
 
            if((index++)==0)
            {
            start=nTemp;
            end=nTemp;
            current=nTemp;
            return index;  
            }          
            nTemp->prev(end);
            end->next(nTemp);
            end=nTemp;
 
 
            return index;              
 
     }
 
 
void* LinkedList::element(unsigned short x)
{
 
 
       rewind();
       for(int i=0;(i<index) && (i<x);i++)  
          {
          current=current->next();
          }
          return current->obj();
          }
 
 

I think the encapsulation routine is much better. Now I can feed it any data type and return the data very quickly casting my data to type as I go. I think anyway, haven't really tested it as it required almost a complete rewrite of the program I had started. Is this more what you had in mind when you spoke of the cessation of serialization in my code?

I'm still kinda working over the bump from c to c++, but I'm really starting to like c++.

[weaknessforcats]

This is pretty good.

However, there is still cross-pollination between LinkedList and Node. That is, you are still using next/prev outside a Node member funciton.

This code:

unsigned short LinkedList::add(void *vPtr)
{

Node *nTemp=new Node(vPtr);

if((index++)==0)
{
start=nTemp;
end=nTemp;
current=nTemp;
return index;
}
nTemp->prev(end);
end->next(nTemp);
end=nTemp;


return index;

}

that adds a Node to the list should be:

void LinkedList::add(void *vPtr)
{
 
            Node *nTemp=new Node(vPtr);
 
            if(start ==0)
            {
               start=nTemp;
               end=nTemp;
               current=nTemp;
               return;
            }          
            end.InsertAfter(nTemp);             
 
            return index;              
 
     }
 

Here the linked list function calls a Node function to do the add.

Node::InsertAfter() would be a member function that inserts the node in the argument after the node used on the call. Maybe:

void Node::InsertAfter(Node* second)
{
        //Case 1: this->next is zero
        if (!this->next)
        {
            //this->prev does not need to change
            this->next = second;
            second->prev = this;
            //second->next does not need to change
            return;
        }
        //Case 2: this->next is not zero
 
 
        second->prev = this;
        second->next = this->next;
 
        this->next->prev  = second;
        this->next = second;
                           //this->prev not changed
 
}
 

You can use this goodie to:

nTemp.InsertAfter(start);      //adds nTemp as Ist node
nPos.InsertAfter(nTemp);    //adds nTemp after the node nPos in the middle
                                         //of the list
 

Then this member function in the LinkedList struct:

void* next() {current=current->next(); return current->obj();}

is really two member functions packed into one:

void Advance();
void* GetData();
 

Here are possible implementations:

void LinkedList::Advance()
{
   this->current = this->current->GetNext();
}
 

where GetNext() is a call to a Node function:

Node* Node::GetNext()
{
       return n->next;
}
 

And to get the value at the current position:

void* LinkedList::GetData()
{
     return this->current->GetData();  //calls Node::GetData()
}
 

where:

void* Node::GetData()
{
     return this->GetData();
}
 

As you can see your code is shaping up very nicely. All of this encapsulation stuff is just experience.